Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes including bugfix for fetching correct contract version at block height #216

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

eelanagaraj
Copy link
Contributor

Description

PR to be rebase/merged, including the following fixes:

  • misc fixes: one-line fixes, removing stale code/nolints, fixing some staticchecks
  • removing the cli block command which, even after fixing, isn't super useful (given that block and network APIs already provided by the Rosetta API) and very error-prone without more fixes (i.e. requiring the correct rosetta db path to be passed in, after fixing that this used a hardcoded path previously)
  • fixing the cli reconcile command which seems useful for reconciling small ranges locally fairly quickly. Likely a useful debugging tool when trying to identify a specific failing block/account. Leaving the reconciliation_debugging.py script as this is easier to quickly modify when debugging specific use cases.
  • fixing an edge case bug where for subaccount txs, the contract address fetched was from the tip of the rosetta node, which is wrong for historical requests where the address was different (i.e. contract not deployed yet, or in the future, in the case of proxy upgrades)

Tested

  • Ran things locally
  • Tested bugfix using cli reconcile

Copy link
Contributor

@palango palango left a comment

Choose a reason for hiding this comment

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

Nice cleanups, thanks for going through all of those!

@eelanagaraj eelanagaraj merged commit f9873b1 into master Sep 28, 2023
4 checks passed
@eelanagaraj eelanagaraj deleted the eelanagaraj/additional-fixes branch September 28, 2023 10:47
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.

2 participants