Skip to content

Conversation

@LucasCambon
Copy link
Collaborator

No description provided.

ETHERSCAN_TOKEN = env.str("ETHERSCAN_TOKEN", None)
ETHERSCAN_DOMAIN = env.str("ETHERSCAN_DOMAIN", "api.etherscan.io")
ETHERSCAN_URL = env.str("ETHERSCAN_URL", "https://{domain}/api?apikey={token}&")
ETHERSCAN_URL = env.str("ETHERSCAN_URL", "https://{domain}/v2/api?apikey={token}&")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acá ya tendría que incluir el chainId también. Que get_etherscan_url lo complete.

tests/test_w3.py Outdated
mocker.patch.dict(
os.environ,
{
"ETHERSCAN_URL": "https://{domain}/v2/api?apikey={token}&",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mejor no sobreescribir la URL para el test, podría enmascarar errores (o cambios que rompen compatibilidad en el futuro)

tox.ini Outdated
testing
deps =
warrant @ git+https://github.com/gnarvaja/warrant.git#egg=warrant
requests-mock
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Por qué esta dependencia que no usamos en ningún lado? Usemos responses. Además la dependencia habría que ponerla en setup.cfg, en la sección de testing.

Si no podés usar cassete (pytest-recording) que ya está incluida, pero capaz para este caso es mejor responses.

if ETHERSCAN_TOKEN is None:
return None
return ETHERSCAN_URL.format(token=ETHERSCAN_TOKEN, domain=ETHERSCAN_DOMAIN)
return ETHERSCAN_URL.format(token=ETHERSCAN_TOKEN, domain=ETHERSCAN_DOMAIN, chainid=ETHERSCAN_CHAIN)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Para mi estaba bien como lo hacia antes, que lo tomaba de self.w3.chain_id

@LucasCambon LucasCambon merged commit c5ec9c2 into main May 22, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants