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

Move network ID from Rosetta to GraphQL #14042

Merged
merged 5 commits into from
Sep 26, 2023

Conversation

Sventimir
Copy link
Contributor

@Sventimir Sventimir commented Sep 6, 2023

We need the node to conform to the Chain Agnostic Standard so that it can be handled by more wallets. To do it, it should return a human-readable identifier of the network it participates in (see CASA namespace for Mina Protocol). The identifier should consist of 2 parts separated by a colon:

  • Cryptocurrency identifier
  • Network identifier

For instance mainnet's identifier should look like this: mina:mainnet. This identifier should be tied down to Rosetta's network identifier.

The query to obtain the identifier is:

query MyQuery {
  networkID
}

To which the daemon will respond for instance:

{
  "data": {
    "networkID": "mina:mainnet"
  }
}

Explain your changes:

  • Reuse ledger.name from the runtime config as network identifier.
  • Fix the namespace to the constant mina.
  • Add GraphQL field networkID returning the ledger name prefixed with constant mina:.
  • Make Rosetta fetch its network_identifier from the daemon.

Explain how you tested your changes:

  • Run the daemon and observe it return the identifier as expected.
  • Run rosetta-cli tests to make sure that Rosetta works properly after this change.

Checklist:

  • Dependency versions are unchanged
    • Notify Velocity team if dependencies must change in CI
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them
  • Closes issue #39 in MinaFoundation repo.

@Sventimir Sventimir requested review from a team as code owners September 6, 2023 13:28
@Sventimir Sventimir self-assigned this Sep 6, 2023
@Sventimir Sventimir requested a review from a team as a code owner September 6, 2023 13:28
@Sventimir
Copy link
Contributor Author

!ci-build-me

Copy link
Member

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

lgtm but you'll have to fix the build (see failing unit test)

@Sventimir
Copy link
Contributor Author

!ci-build-me

1 similar comment
@Sventimir
Copy link
Contributor Author

!ci-build-me

@mrmr1993
Copy link
Member

mrmr1993 commented Sep 8, 2023

!approved-for-mainnet

@Sventimir
Copy link
Contributor Author

!ci-build-me

3 similar comments
@Sventimir
Copy link
Contributor Author

!ci-build-me

@Sventimir
Copy link
Contributor Author

!ci-build-me

@Sventimir
Copy link
Contributor Author

!ci-build-me

@Sventimir
Copy link
Contributor Author

!ci-builld-me

@bkase
Copy link
Member

bkase commented Sep 13, 2023

!ci-build-me

@Sventimir Sventimir force-pushed the sventimir/graphql-network-id branch 2 times, most recently from 1a34833 to d2e5fc0 Compare September 13, 2023 12:40
@Sventimir
Copy link
Contributor Author

!ci-build-me

1 similar comment
@Sventimir
Copy link
Contributor Author

!ci-build-me

@Sventimir Sventimir requested a review from a team as a code owner September 14, 2023 13:46
@Sventimir
Copy link
Contributor Author

!ci-build-me

1 similar comment
@Sventimir
Copy link
Contributor Author

!ci-build-me

@Sventimir Sventimir force-pushed the sventimir/graphql-network-id branch from 7dbf83a to a2c110d Compare September 15, 2023 11:13
The query returns the chain-agnostic identifier of the network
conforming with the standard https://github.com/ChainAgnostic/namespaces.
@Sventimir Sventimir force-pushed the sventimir/graphql-network-id branch from a2c110d to b855c0a Compare September 15, 2023 11:14
@Sventimir
Copy link
Contributor Author

!ci-build-me

@Sventimir
Copy link
Contributor Author

!ci-build-me

@mrmr1993
Copy link
Member

!approved-for-mainnet

@Sventimir Sventimir merged commit be93a75 into berkeley Sep 26, 2023
2 checks passed
@Sventimir Sventimir deleted the sventimir/graphql-network-id branch September 26, 2023 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants