Repository navigation
feat: pda upgrade poc - #135
s2imonovic wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| // Initializes the gateway PDA. | ||
| pub fn initialize(ctx: Context<Initialize>, tss_address: [u8; 20], chain_id: u64) -> Result<()> { | ||
| let initialized_pda = &mut ctx.accounts.pda; | ||
| let bump = ctx.bumps.pda; |
There was a problem hiding this comment.
lets also add other issues that this might be closing, all issues like bump and authority that required pda upgrade to work
i also think we can use this opportunity to put deposit fee in pda instead of constant and add posibility to update it
| // Migrates existing PDA to new format by reallocating account space. | ||
| // This is a one-time migration that can be called after program upgrade. | ||
| pub fn migrate_pda(ctx: Context<MigratePda>) -> Result<()> { | ||
| let pda_info = &ctx.accounts.pda.to_account_info(); |
There was a problem hiding this comment.
dont we need verify_authority here as well?
| let new_size = 8 + std::mem::size_of::<Pda>(); | ||
|
|
||
| // If already migrated (size matches new size), do nothing | ||
| if current_size >= new_size { |
There was a problem hiding this comment.
do we need this check or it will be automatically checked by anchor somehow?
| return Ok(()); | ||
| } | ||
|
|
||
| // The realloc constraint in the context has already resized the account |
There was a problem hiding this comment.
do we need to make sure that pda with more data inside will contain enough funds for rent exempt? maybe that should be documented somewhere, even though it will probably have enough since deposits go to pda anywayrs
| pda.nominated_authority = None; | ||
| } | ||
|
|
||
| msg!("PDA migrated: bump = {}, new_size = {}", bump, new_size); |
There was a problem hiding this comment.
should we add nominated authority as well to log?
| expect(pdaAfter.depositPaused).to.equal(pdaBefore.depositPaused); | ||
| }); | ||
|
|
||
| it("Should be idempotent if migration called twice", async () => { |
There was a problem hiding this comment.
from comment above, here you can check that size didnt change for example
| expect(pdaAfter.nominatedAuthority).to.be.null; | ||
| }); | ||
|
|
||
| it("Should use stored bump instead of recomputing", async () => { |
There was a problem hiding this comment.
bump will always be the same for seed + program id if im not wrong, so this tests seems like its not needed
| // Verify state is unchanged | ||
| const pdaAfter = await gatewayProgram.account.pda.fetch(pdaAccount); | ||
| expect(pdaAfter.bump).to.equal(bumpBefore); | ||
| expect(pdaAfter.nominatedAuthority).to.be.null; |
There was a problem hiding this comment.
please also add test to check if only authority can migrate pda, since that is also missing in gateway function
|
|
||
| // Verify PDA structure is intact after migration | ||
| const pda = await gatewayProgram.account.pda.fetch(pdaAccount); | ||
| expect(pda.bump).to.be.a("number"); |
There was a problem hiding this comment.
can these checks be more specific?
| expect(pda.chainId).to.not.be.undefined; | ||
| }); | ||
|
|
||
| it("Should work with SPL token operations after migration", async () => { |
There was a problem hiding this comment.
we can do checks with deposit and withdraw after migration to be sure it works
also this migration should be somehow in zetae2e upgrade tests but that is separate PR
Closing: zeta-chain/node#3324