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

Extend Rosetta to work with MyCelo testnets #201

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

eelanagaraj
Copy link
Contributor

@eelanagaraj eelanagaraj commented Aug 7, 2023

Description

  • Adds geth.networkid flag
    • This is nessary when genesis ChainID != networkID, as is the case for some mycelo-based testnets
  • Adds functionality to fetch necessary state for Rosetta (core contract addresses, GPM, CarbonOffsetPartner) from a static block when initializing the monitor & specifically prompted via --monitor.initcontracts; note this is not done by default.
    • This is necessary for mycelo testnets where this information is already configured at genesis. (Rosetta by default only listens for updates which is not sufficient if there is initial state at genesis.) While this is primarily useful for testing with mycelo-based testnets right now, this should also help with future L2 changes.
    • Design note: this follows the structure of block_processor. A larger refactor is possible here but I felt that to be overkill for now (especially since we will need to make more sweeping changes for L2 work in the future) and would unnecessarily introduce possible complexity/changes to a relatively stable part of the Rosetta service.

Tested

  • Manually tested this:
    • using the globaltestnet (with longer reconciliation tests)
    • spun up a 1-validator mycelo testnet and ran a rosetta node against this

@palango
Copy link
Contributor

palango commented Aug 8, 2023

The changes look good, I'm just wondering if we can remove the --monitor.initcontracts flag and instead somehow detect if the initial state is set in the genesis block. But maybe that's not worth the effort.

@eelanagaraj
Copy link
Contributor Author

Thanks! and yeah I couldn't find a non-hacky way immediately (and wanted to make sure this minimally affects existing rosetta, i.e. needing to explicitly turn this on) but I imagine with L2 stuff in the future we can spend more time finding a way to do that! If any ideas occur to you later, happy to update!

@eelanagaraj eelanagaraj force-pushed the eelanagaraj/tobin-tax-removal branch 2 times, most recently from f938403 to c2c0e92 Compare August 23, 2023 12:32
Base automatically changed from eelanagaraj/tobin-tax-removal to master August 23, 2023 12:55
@eelanagaraj eelanagaraj marked this pull request as ready for review August 23, 2023 13:03
README.md Show resolved Hide resolved
@eelanagaraj eelanagaraj merged commit b271ae0 into master Aug 24, 2023
4 checks passed
@eelanagaraj eelanagaraj deleted the eelanagaraj/run-on-mycelo branch August 24, 2023 13:36
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