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

Makes network=mainnet work properly in Rosetta #8877

Merged
merged 3 commits into from May 20, 2021
Merged

Conversation

bkase
Copy link
Member

@bkase bkase commented May 13, 2021

When Rosetta is built with the mainnet profile, report (correctly) that the network being used in mainnet.

@bkase bkase changed the title Attempts to make network=mainnet in rosetta Makes network=mainnet work properly in Rosetta May 13, 2021
@bkase bkase added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label May 13, 2021
@jspada jspada requested review from jspada and psteckler May 13, 2021 18:29
@bkase bkase changed the base branch from develop to compatible May 13, 2021 18:37
@bkase
Copy link
Member Author

bkase commented May 13, 2021

@jspada is testing this against the new ledger build now!

Copy link
Contributor

@jspada jspada left a comment

Choose a reason for hiding this comment

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

Works for me!

mainnet

REQUEST = {"metadata": {}}

Getting network identifier... mainnet

RESPONSE = {"network_identifiers": [{"blockchain": "coda", "network": "mainnet"}]}

testnet

REQUEST = {"metadata": {}}

Getting network identifier... testnet

RESPONSE = {"network_identifiers": [{"blockchain": "coda", "network": "testnet"}]}

src/app/rosetta/lib/network.ml Outdated Show resolved Hide resolved
Copy link
Member

@mrmr1993 mrmr1993 left a comment

Choose a reason for hiding this comment

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

Code looks good; I haven't tested, so I'll assume that you or @jspada will.

@jspada
Copy link
Contributor

jspada commented May 16, 2021

Slightly different behaviour. In Brandon's previous fix it used to report testnet when rosetta was built with --profile=testnet_postake_medium_curves and mainnet when rosetta was built with --profile=mainnet

In the new version of the fix it seems depend on what profile mina is built with (rather than rosetta) and reports debug or mainnet instead.

$ dune build src/app/cli/src/mina.exe --profile=mainnet
$ test-rosetta
REQUEST = {"metadata": {}}
RESPONSE = {"network_identifiers": [{"blockchain": "coda", "network": "mainnet"}]}
$ dune build src/app/cli/src/mina.exe --profile=testnet_postake_medium_curves
$ test-rosetta
REQUEST = {"metadata": {}}
RESPONSE = {"network_identifiers": [{"blockchain": "coda", "network": "debug"}]}

Is this the desired behaviour?

@bkase bkase merged commit b976175 into compatible May 20, 2021
@bkase bkase deleted the make-network-mainnet branch May 20, 2021 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants