Refactor methods into versioned module [NONEVM-1114] #414
Conversation
| @@ -1857,6 +1857,114 @@ | |||
| ] | |||
| } | |||
| }, | |||
| { | |||
| "name": "LockOrBurnInV1", | |||
There was a problem hiding this comment.
Note for reviewers: There is no real IDL changes due to this PR, it's just a reordering of some structs that I'm moving to different files so the auto-generated IDL lists them in different order
| }); | ||
|
|
||
| Ok(()) | ||
| v1::tokens::register_token_admin_registry_via_get_ccip_admin( |
There was a problem hiding this comment.
nit: rename tokens to token-admin-registry? And then fee_quoter could be a part of it as well
There was a problem hiding this comment.
I'd rather keep fee_quoter separate from the rest of the tokens logic, as the fee calculation will have a lot of code unrelated to general token management. I'll make the rename though 👍
| mod fee_quoter; | ||
| use crate::fee_quoter::*; | ||
| mod instructions; | ||
| use crate::instructions::*; | ||
|
|
||
| // Anchor discriminators for CPI calls | ||
| const CCIP_RECEIVE_DISCRIMINATOR: [u8; 8] = [0x0b, 0xf4, 0x09, 0xf9, 0x2c, 0x53, 0x2f, 0xf5]; // ccip_receive |
There was a problem hiding this comment.
Is this var then imported from the instructions crate?
There was a problem hiding this comment.
Yes, the new offramp.rs file imports this. All the consts defined in the lib.rs are used in either the onramp or the offramp now, but they are tied to our interface with receivers and tokenpools so the constants themselves shouldn't be versioned.
| use anchor_lang::prelude::*; | ||
| use ethnum::U256; | ||
|
|
||
| use crate::{ocr3base::Ocr3Report, ToTxData, TOKENPOOL_LOCK_OR_BURN_DISCRIMINATOR}; | ||
| // TODO this file has to be broken up |
There was a problem hiding this comment.
Public interfaces that should remain the same, for example Solana2Any and Any2Solana should still be part of this module, right? And not part of the instructions crate
There was a problem hiding this comment.
Right, anything that affects the public interface won't be versioned. That's why I have to go through this file with a finer comb and split the code there, as some interface-related structs also have methods that should be upgradeable, so I'll have to make some changes here and there to achieve that. As mentioned in the PR description, I'll do that in a separate PR, to not make this grow indefinitely
| use anchor_spl::token_interface; | ||
|
|
||
| use crate::{ | ||
| AcceptOwnership, AddBillingTokenConfig, AddChainSelector, BillingTokenConfig, CcipRouterError, |
There was a problem hiding this comment.
Is there any cyclic dependency between files?
There was a problem hiding this comment.
Mostly no. There is only one cycle right now, which is easy to break if you prefer.
The lib.rs is the only non-versioned file that imports versioned code. But the versioned code also imports the errors and constants that are listed in the lib.rs. This is not a problem in Rust, as I've tested this code locally and it's passing CI, but then again it's trivial for me to move errors and constants to separate files if you'd prefer.
| @@ -0,0 +1,58 @@ | |||
| use anchor_lang::prelude::*; | |||
There was a problem hiding this comment.
It looks like there is a typo in the file name, right? This should be something related to pools
There was a problem hiding this comment.
Well, I extracted these from the messages.rs so I kept the name. I get that they are related to pools, so I'm happy to move them there if you think that's better, but maintaining the name was the "safer" refactor given that I haven't dived too deep into how those structs are used
…o refactor/part-1
…nk-ccip into refactor/part-1
|
This is the first part of a refactor, more PRs to follow. The goal is to have a versioned module with just logic (no state or external interface) so that it can be more easily upgradeable.
This PR moves all methods into an
instructions::v1module, so thelib.rsexposes the external interface and just invokes the logic in said module. Upcoming PRs will continue working on other shared files, such asmessages.rsandocr3base.rswhich have a mix of external interfaces and internal logic.