- 
                Notifications
    You must be signed in to change notification settings 
- Fork 0
chore: rust bindings from source contract #33
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
Conversation
| SEQ-117 Handle transactions that must be split up into parts
 The max size of a Solidity transaction is 128KB, but we're limited to ~90KB of data due to the overhead in formatting a bulk smart contract call. We need to split transactions that are too large up into parts. See https://ethereum.stackexchange.com/a/144128 for the 128KB limit in Solidity and SEQ-103 for the 90KB limit in our smart contracts. This should be done in a way that if the max gas changes (because e.g. more modules are added, lowering the room we have for events), the system seamlessly adapts and handles the lower max limit. We can't assume that the max limit will be a particular KB threshold, since we can't know what modules are in use (and therefore how much gas/throughput we'll have available). Note that we will also be compressing transaction data using brotli compression, and compressing prior to chunking will likely be more fruitful than chunking then compression. The backfill project is using this for their TX lists, and we can reuse the same logic. Acceptance criteria: 
 | 
| Oh cool! One thing to note here is that making changes to Solidity contracts now breaks the Rust codebase, which may impose some coordination overhead - changing the smart contract interface breaks the build and prevents merging the changes until the Rust codebase is also updated. As a stylistic nit, I'd prefer to keep the implementation out of the  | 
| 
 Good callout @RomanHodulak . That's a bummer and imo we shouldn't impose this coupling on the rest of the team. I do think there is value in generating the Rust bindings directly from the source contract. Potential solution 
 Can you think of any other ways to handle this? EDIT: | 
92f0d08    to
    cc2a958      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool macro! Looks much simpler now
| use alloy::sol; | ||
| sol!( | ||
| #[sol(rpc)] | ||
| "../../metabased-contracts/src/MetabasedSequencerChain.sol" | ||
| ); | ||
| pub use MetabasedSequencerChain as MetabasedSequencerChainInstance; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool! I like how it handles automatic ABI generation
We should document this functionality by linking to https://alloy.rs/highlights/the-sol!-procedural-macro.html, since people new to the codebase are likely unfamiliar with the sol! macro
| 
 There is another option, which is to always version our contracts on the Solidity side. We've done this for a while, since you often need older versions of contracts for verification purposes. Once a contract is "released" we can move it into a subdirectory where it remains versioned and is always unmodified. In that case,  | 
| @gustavoguimaraes Do you have thoughts on the versioning questions above? | 
| 
 I like the release version idea you proposed @WillPapper . I can start doing it one this PR is merged. | 
| The metabased-contracts is the upstream and the metabased-sequencer is the downstream for sequencer chain solidity contract. We want to decouple the versioning of upstream and downstream. This can be achieved by: Generating the rust bindings using: 
 The main issue with 1. is duplication of knowledge - since we're keeping the contract interface on two places that need to be in sync. Although nice thing is that we can declare only what we need and since we're making it by hand it is forcing us to think about the changes being made and what needs to be updated on call sites. This issue is solved in 2. as we get one source of truth / knowledge. We can observe changes and go through them by comparing the release source files. Although we need to filter out changes not relevant to the downstream code. | I like the release version idea you proposed @WillPapper . I can start doing it one this PR is merged. @gustavoguimaraes I think it would be better to first make the immutable releases and then followup with PR on the rust side that consumes it. I don't think we need to merge this PR in this state nor that it blocks this effort. Wdyt? | 
| 
 Sounds good @RomanHodulak 👍 | 
| HOLD on this until contract updates are immutable+versioned | 
| 
 chatted with @gustavoguimaraes and he'll work on a PR to solve for this while also avoiding repo bloat from old contract versions | 
| @WillPapper suggested using a specific commit hash of the contracts here so let's try and use that | 
| we're currently generating bindings for these contracts here: please take it into account. We might want to move them to a more common place at the project root that both translator and sequencer can access. | 
Ticket
What does this PR do?
Does this PR introduce any breaking changes (API/schema)?
Do any environment variables need to be updated or added before deployment?
How can this PR be tested?