Fixes for etherscan #8346
Fixes for etherscan #8346
Conversation
yabirgb
commented
Jul 30, 2024
yabirgb
commented
- Fail fast with etherscan in ethereum
- Avoid querying a evm chain if there are no accounts for it
- ethereum nodes to connect without addresses
rotkehlchen/chain/aggregator.py
Outdated
| @@ -544,6 544,9 @@ def query_balances( | |||
| xpub_manager.check_for_new_xpub_addresses(blockchain=blockchain) # type: ignore # is checked in the if | |||
| else: # all chains | |||
| for chain in SupportedBlockchain: | |||
| if chain.is_evm() and len(self.accounts.get(chain)) == 0: | |||
There was a problem hiding this comment.
eth2 is an exception since we don't have a balance structure for them and bitcoin does the detection of new addresses on xpub. If we change it we can adjust it. But both eth2 and btc do check before executing logic if there are addresses added
yabirgb
force-pushed
the
fixes-etherscan
branch
3 times, most recently
from
743d29f
to
af8ec47
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## bugfixes #8346 /- ##
============================================
Coverage 53.77% 53.78% 0.01%
============================================
Files 1708 1709 1
Lines 168799 168952 153
Branches 13951 13986 35
============================================
Hits 90769 90879 110
- Misses 75611 75647 36
- Partials 2419 2426 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Etherscan requires an API key in ethereum to perform queries in the RPC endpoint. Instead of making a request and wait for it to fail raise remoteError before querying
yabirgb
force-pushed
the
fixes-etherscan
branch
2 times, most recently
from
994c0ad
to
3517d89
Compare
| api_key = ApiKey(raw_key) # we warn right before that a key is needed | ||
| log.debug(f'Using default etherscan key for {self.chain}') | ||
| else: | ||
| self.msg_aggregator.add_error(msg := f'Etherscan for {self.chain} was queried but requires an API key. Please add one.') # noqa: E501 |
There was a problem hiding this comment.
not sure I get this. What we need to achieve is:
- If there is no api key use default. But warn user as we do now that they need an api key
- If there is an api key from the user use that.
There are several steps for the different chains and not in all of them we are checking if the number of accounts is 0. Check it before calling the logic of the EVM chain.
|
|
||
| api_key = ApiKey(ROTKI_INCLUDED_KEYS[self.chain]) | ||
| log.debug(f'Using default etherscan key for {self.chain}') |
There was a problem hiding this comment.
Do we need this? Won';t this spam the logs? We can probably see that the default key is being used as api key is also logged in the logs right?
On the other hand seeing this ins the logs would point us to a user who needs to take action. Not sure what's best.
LefterisJP
temporarily deployed
to
cassette-merge
GitHub Actions
Inactive
|
rotki/test-caching/tree/fixes-etherscan was successfully merged |
