solana: reclaim rent for old accounts by kcsongor · Pull Request #4746 · wormhole-foundation/wormhole

kcsongor · 2026-04-10T15:03:46Z

No description provided.

This is implemented as an opt-in feature.

johnsaigle · 2026-04-10T15:09:50Z

@claude

(I think this is broken unfortunately, but giving it a whirl)

github-actions · 2026-04-10T15:10:09Z

Claude Code is working…

I'll analyze this and get back to you.

View job run

Copilot

Pull request overview

This PR adds rent-reclamation capabilities to the Solana bridge program by introducing close instructions for old message/VAA-related accounts (with a 30-day retention policy), and extends the solitaire! macro to optionally support Anchor-style event CPI dispatch guarding (enabled for the bridge program).

Changes:

Add optional Anchor event CPI dispatch guard to solitaire! and enable it for the bridge program.
Introduce close_posted_message and close_signature_set_and_posted_vaa instructions to reclaim rent, plus supporting error variants and instruction builders.
Populate submission_time for PostedVAA and expand integration tests/helpers to validate retention boundaries and rent reclamation flows.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
solana/solitaire/program/src/macros.rs	Adds `@event_cpi` mode and a dispatch guard for Anchor event CPI selector.
solana/solitaire/program/src/lib.rs	Defines event CPI tag/seed constants used by the guard and bridge program.
solana/bridge/program/src/lib.rs	Enables event CPI support and wires new close instructions into dispatch.
solana/bridge/program/src/api.rs	Exposes the new close APIs.
solana/bridge/program/src/api/close_posted_message.rs	Implements posted message closure with retention checks and event emission.
solana/bridge/program/src/api/close_signature_set_and_posted_vaa.rs	Implements signature set + posted VAA closure logic with retention/guardian-set checks.
solana/bridge/program/src/api/post_vaa.rs	Sets `submission_time` when posting VAAs; exposes `check_active` for reuse.
solana/bridge/program/src/instructions.rs	Adds client-side instruction constructors for the new close instructions.
solana/bridge/program/src/error.rs	Adds new program error variants for close/retention/event validation.
solana/bridge/program/tests/common.rs	Switches to `ProgramTestContext` and adds helpers for close instructions + patching `submission_time`.
solana/bridge/program/tests/integration.rs	Refactors test harness and adds extensive tests for event CPI guard and rent reclamation behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T15:14:30Z

+        data: ix_data,
+    };
+
+    invoke_signed(&ix, ctx.accounts, &[&[EVENT_AUTHORITY_SEED, &[bump]]])?;


invoke_signed is called with ctx.accounts, but the CPI instruction declares only one account (expected_authority). The AccountInfos passed to invoke_signed must be in the same order/shape as ix.accounts, otherwise the CPI will fail due to account mismatch. Pass only the event authority AccountInfo (or update ix.accounts to match the provided slice) so the first account is the PDA signer expected by the event guard.

Suggested change

invoke_signed(&ix, ctx.accounts, &[&[EVENT_AUTHORITY_SEED, &[bump]]])?;

invoke_signed(

&ix,

&[event_authority.clone()],

&[&[EVENT_AUTHORITY_SEED, &[bump]]],

)?;

Copilot · 2026-04-10T15:14:31Z

+        let message_data = MessageData::try_from_slice(&data[3..])
+            .map_err(|_| SolitaireError::ProgramError(ProgramError::InvalidAccountData))?;
+
+        // Check retention: submission_time must be at or before (now - 30 days).
+        if (message_data.submission_time as i64) > accs.clock.unix_timestamp - RETENTION_PERIOD {
+            return Err(MessageWithinRetentionWindow.into());
+        }


The retention check treats submission_time == 0 as "very old" and will allow immediate closure. If 0 is used by legacy accounts created before submission_time was populated, this bypasses the intended 30-day grace period (note the VAA close path handles this via DEFAULT_SUBMISSION_TIME). Consider applying the same default/grace logic here.

Copilot · 2026-04-10T15:14:31Z

+            if $a.is_empty() || !$a[0].is_signer {
+                return Err(SolitaireError::InvalidSigner(
+                    solana_program::pubkey::Pubkey::default(),
+                ));
+            }


When the account list is non-empty but the first account is not a signer, this returns InvalidSigner(Pubkey::default()), which loses the offending key and makes debugging harder. Consider returning InvalidSigner(*a[0].key) when available, and only use default when the slice is empty.

Suggested change

if $a.is_empty() || !$a[0].is_signer {

return Err(SolitaireError::InvalidSigner(

solana_program::pubkey::Pubkey::default(),

));

}

if $a.is_empty() {

return Err(SolitaireError::InvalidSigner(

solana_program::pubkey::Pubkey::default(),

));

}

if !$a[0].is_signer {

return Err(SolitaireError::InvalidSigner(*$a[0].key));

}

Copilot · 2026-04-10T15:14:31Z

+    let message_lamports =
+        common::get_account_balance(&mut test_ctx.banks_client, message_key).await;
+
+    // Set submission_time to 0 (epoch) so it passes the 30-day retention check.


Comment mismatch: this sets submission_time to old_submission_time() (now - 31 days), not 0/epoch. Please update the comment to reflect what the test is doing.

Suggested change

// Set submission_time to 0 (epoch) so it passes the 30-day retention check.

// Set submission_time to an old value (via old_submission_time(), e.g. now - 31 days)

// so it passes the 30-day retention check.

Copilot · 2026-04-10T15:14:32Z

+            .unwrap()
+            .unwrap();


account_data.data[41..45] will panic with an out-of-bounds slice if the account is missing or the layout changes. Adding a length check (and ideally validating the 3-byte prefix) would make failures clearer and avoid hard-to-diagnose panics in tests.

Suggested change

.unwrap()

.unwrap();

.expect("failed to fetch account for submission_time patch")

.expect("cannot patch submission_time: account does not exist");

assert!(

account_data.data.len() >= 45,

"cannot patch submission_time: account data too short (expected at least 45 bytes, got {})",

account_data.data.len()

);

assert!(

matches!(&account_data.data[..3], b"msg" | b"msu" | b"vaa"),

"cannot patch submission_time: unexpected account prefix {:?}",

&account_data.data[..3]

);

Copilot · 2026-04-10T15:14:32Z

+        // Close PostedVAA: transfer lamports to fee_collector.
+        let vaa_lamports = accs.posted_vaa.lamports();
+        **accs.fee_collector.lamports.borrow_mut() += vaa_lamports;
+        **accs.posted_vaa.lamports.borrow_mut() = 0;
+        accs.posted_vaa.data.borrow_mut().fill(0);


Lamports are accumulated with += without overflow checking. Elsewhere the program uses checked arithmetic (e.g., MathOverflow) for lamport math; consider using checked_add here as well to avoid potential wraparound.

Copilot · 2026-04-10T15:14:32Z

+    // 6. Close signature_set: transfer lamports to fee_collector.
+    let sig_lamports = accs.signature_set.lamports();
+    **accs.fee_collector.lamports.borrow_mut() += sig_lamports;
+    **accs.signature_set.lamports.borrow_mut() = 0;
+    accs.signature_set.data.borrow_mut().fill(0);


Lamports are accumulated with += without overflow checking. Elsewhere the program uses checked arithmetic (e.g., MathOverflow) for lamport math; consider using checked_add here as well to avoid potential wraparound.

Copilot · 2026-04-10T15:14:32Z

+) -> Result<()> {
+    // Verify self_program is actually our program.
+    if self_program.key != ctx.program_id {
+        return Err(InvalidEventAuthority.into());


This branch checks self_program.key != ctx.program_id but returns InvalidEventAuthority, which is misleading (the program account is what's invalid here, not the event authority). Consider returning a more accurate error variant for easier diagnosis.

Suggested change

return Err(InvalidEventAuthority.into());

return Err(InvalidProgramOwner.into());

changes made

The main challenge in doing this is figuring out when we're on testnet, since the solana contract only indirectly encodes this by taking the contract address as an env variable. The approach we take is minimally invasive in that it doesn't require introducing a new flag. Instead, we just check that the contract address is the testnet address.

aleksiko17 · 2026-04-20T16:14:42Z

I staked my W coins a few months ago on the official website https://w.wormhole.com in the Solana network. Everything was displaying correctly before, but now my staked coins are no longer visible in the dashboard. What could be the reason?

kcsongor added 2 commits April 10, 2026 16:56

solana/solitaire: add support for anchor-style event cpi

0419bbc

This is implemented as an opt-in feature.

solana/bridge: store VAA submission time

f1727e9

kcsongor requested a review from Copilot April 10, 2026 15:04

Copilot started reviewing on behalf of kcsongor April 10, 2026 15:05 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

johnsaigle reviewed Apr 10, 2026

View reviewed changes

Comment thread solana/bridge/program/src/api/close_signature_set_and_posted_vaa.rs

Comment thread solana/bridge/program/src/api/close_posted_message.rs

Comment thread solana/bridge/program/src/api/close_posted_message.rs

kcsongor force-pushed the solana/reclaim-rent branch 2 times, most recently from 21aeea0 to 40562e5 Compare April 13, 2026 15:06

solana: close old posted_message, signature_set, and posted_vaa accounts

e3bb812

kcsongor force-pushed the solana/reclaim-rent branch from 40562e5 to e3bb812 Compare April 13, 2026 16:08

evan-gray previously requested changes Apr 14, 2026

View reviewed changes

Comment thread solana/bridge/program/src/api/close_posted_message.rs Outdated

evan-gray mentioned this pull request Apr 14, 2026

feat(node): solana core close event handling wormholelabs-xyz/wormhole#17

Draft

solana/bridge: emit entire account in close_posted_message

53bbde7

	// Set submission_time to 0 (epoch) so it passes the 30-day retention check.
	// Set submission_time to an old value (via old_submission_time(), e.g. now - 31 days)
	// so it passes the 30-day retention check.

-            .unwrap()
-            .unwrap();
+            .expect("failed to fetch account for submission_time patch")
+            .expect("cannot patch submission_time: account does not exist");
+        assert!(
+            account_data.data.len() >= 45,
+            "cannot patch submission_time: account data too short (expected at least 45 bytes, got {})",
+            account_data.data.len()
+        );
+        assert!(
+            matches!(&account_data.data[..3], b"msg" | b"msu" | b"vaa"),
+            "cannot patch submission_time: unexpected account prefix {:?}",
+            &account_data.data[..3]
+        );

	return Err(InvalidEventAuthority.into());
	return Err(InvalidProgramOwner.into());

Conversation

kcsongor commented Apr 10, 2026

Uh oh!

johnsaigle commented Apr 10, 2026

Uh oh!

github-actions Bot commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aleksiko17 commented Apr 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants