Skip to content

Conversation

amirylm
Copy link

@amirylm amirylm commented Nov 1, 2024

Introducing Rust code generation.

  • generalized code for go templates
  • added rs folder for rust:
    • (golang) code and template for generating the rust module
    • generated rust module chainselectors with tests

NOTES:

  • aptos/solana for the rust implementation are out of the scope of this PR
  • rust toolchain is a soft requirement, docker container is used as a fallback if rust is not installed

Copy link

@PabloMansanet PabloMansanet left a comment

Choose a reason for hiding this comment

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

Looking good! Just left a few style suggestions and some mild safety ones, but nothing that's a showstopper (though I think anyhow -> thiserror is definitely worth doing).

I suppose there are ways we could reduce the boilerplate by storing the string representation, ID and selector in a const array or something similar at the top, but I don't think it's worth it to be stingy with the LOC of a generated file, so this approach looks good to me.

Debug, Hash, PartialEq, Eq, Clone, Copy, serde::Serialize, serde::Deserialize, PartialOrd, Ord,
)]
#[repr(u64)]
pub enum ChainName {

Choose a reason for hiding this comment

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

I may be bikeshedding here, but is this really a ChainName? I would think this enum would be the Chain, and that Name would specifically refer to the stringified version of it.

Copy link
Author

Choose a reason for hiding this comment

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

We inherit the naming, we use this type to read strings from configs or keystore (i.e. actual chain names).

Agree that now might be a good time to align

PabloMansanet
PabloMansanet previously approved these changes Nov 5, 2024
Copy link

@PabloMansanet PabloMansanet left a comment

Choose a reason for hiding this comment

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

Looks good to me now! 👍

@amirylm amirylm marked this pull request as ready for review January 9, 2025 14:16
@amirylm amirylm requested review from a team as code owners January 9, 2025 14:16
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