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

[Feature]: Make Rosetta only get compiled under a build flag #16402

Closed
ValarDragon opened this issue Jun 1, 2023 · 4 comments
Closed

[Feature]: Make Rosetta only get compiled under a build flag #16402

ValarDragon opened this issue Jun 1, 2023 · 4 comments

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Jun 1, 2023

Summary

Right now all node binaries contain Rosetta code. Ideally we can get the rosetta code to be under a build flag, and not built by default for most full nodes. This should give the following benefits to nodes:

  • No Rosetta command by default in osmosisd. (If build flag not present, no command is added in the respective function call)
  • Slightly faster build times. (Gets rid of many of the rosetta cryptography dependencies, and the Rosetta codebase itself looks not-small)
  • Slightly smaller binaries

Problem Definition

Advantages are listed above. The biggest one to me is the root of the CLI being simpler. Faster build times would be nice if it actually holds in practice.

Proposal

  • Add a build flag for all the Rosetta code, e.g. ROSETTA
  • if !ROSETTA, only compile this function (RosettaCommand)
    func RosettaCommand(ir codectypes.InterfaceRegistry, cdc codec.Codec) *cobra.Command {
    as a no-op.
  • Add a makefile command for install-rosetta to be easier for people to self-discover there is rosetta support. (It will just run something equivalent to ROSETTA=true make install
@julienrbrt
Copy link
Member

julienrbrt commented Jun 1, 2023

This seems like something that should be done at the app level? Not adding the command when using a specific build tag.
Chains are already free to not include rosetta in their root.go (https://github.com/cosmos/cosmos-sdk/blob/v0.47.2/simapp/simd/cmd/root.go#L186-L187).

btw, rosetta will most probably be extracted somewhere else: #16394, so the flow may be a bit weird if it isn't done at app level.

@tac0turtle
Copy link
Member

tac0turtle commented Jun 12, 2023

rosetta will be pulled out of the binary and only run in standalone mode going forward. Closing this for now, also this issue is more for apps than the sdk as if we do the flag apps will need to do it as well

@ValarDragon
Copy link
Contributor Author

ValarDragon commented Jun 12, 2023

You can have the build flag at the rosetta library, so then the flag is standardized no?

How do you get convergent standards, without it being in the rosetta source repo, and demonstrated in the example app?

@tac0turtle
Copy link
Member

How do you get convergent standards, without it being in the rosetta source repo, and demonstrated in the example app?

The standards supported by rosetta are only for chains they would like to implement, otherwise all chains have to implement their own that work from the types provided. Is that what you are asking?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants