diff --git a/solana/bridge/program/src/api/governance.rs b/solana/bridge/program/src/api/governance.rs index 0978c59c9..5195dc917 100644 --- a/solana/bridge/program/src/api/governance.rs +++ b/solana/bridge/program/src/api/governance.rs @@ -3,6 +3,7 @@ use solitaire::*; use solana_program::{ program::invoke_signed, pubkey::Pubkey, + sysvar::rent::Rent, }; use solitaire::{ processors::seeded::Seeded, @@ -28,6 +29,7 @@ use crate::{ Error::{ InvalidFeeRecipient, InvalidGovernanceKey, + InvalidGovernanceWithdrawal, InvalidGuardianSetUpgrade, }, CHAIN_ID_SOLANA, @@ -222,6 +224,9 @@ pub struct TransferFees<'b> { /// Fee recipient pub recipient: Mut>, + + /// Rent calculator to check transfer sizes. + pub rent: Sysvar<'b, Rent>, } impl<'b> InstructionContext<'b> for TransferFees<'b> { @@ -244,6 +249,15 @@ pub fn transfer_fees( ) -> Result<()> { verify_claim(&accs.vaa)?; + if accs + .fee_collector + .lamports() + .saturating_sub(accs.vaa.amount.as_u64()) + < accs.rent.minimum_balance(accs.fee_collector.data_len()) + { + return Err(InvalidGovernanceWithdrawal.into()); + } + accs.vaa.claim(ctx, accs.payer.key)?; // Transfer fees diff --git a/solana/bridge/program/src/instructions.rs b/solana/bridge/program/src/instructions.rs index 1e09d922d..fe7ba872c 100644 --- a/solana/bridge/program/src/instructions.rs +++ b/solana/bridge/program/src/instructions.rs @@ -361,6 +361,7 @@ pub fn transfer_fees( AccountMeta::new(claim, false), AccountMeta::new(fee_collector, false), AccountMeta::new(recipient, false), + AccountMeta::new_readonly(sysvar::rent::id(), false), AccountMeta::new_readonly(solana_program::system_program::id(), false), ], diff --git a/solana/bridge/program/src/lib.rs b/solana/bridge/program/src/lib.rs index 5445c4732..c402a0b47 100644 --- a/solana/bridge/program/src/lib.rs +++ b/solana/bridge/program/src/lib.rs @@ -63,14 +63,15 @@ enum Error { InvalidGovernanceChain, InvalidGovernanceKey, InvalidGovernanceModule, + InvalidGovernanceWithdrawal, InvalidGuardianSetUpgrade, InvalidHash, InvalidSecpInstruction, MathOverflow, PostVAAConsensusFailed, PostVAAGuardianSetExpired, - VAAAlreadyExecuted, TooManyGuardians, + VAAAlreadyExecuted, } /// Translate from program specific errors to Solitaire framework errors. Log the error on the way diff --git a/solana/bridge/program/tests/integration.rs b/solana/bridge/program/tests/integration.rs index 402d16fea..dd3331706 100644 --- a/solana/bridge/program/tests/integration.rs +++ b/solana/bridge/program/tests/integration.rs @@ -148,6 +148,7 @@ fn run_integration_tests() { test_transfer_fees(&mut context); test_transfer_fees_fails(&mut context); test_transfer_too_much(&mut context); + test_transfer_total_fails(&mut context); } fn test_initialize(context: &mut Context) { @@ -1180,3 +1181,62 @@ fn test_foreign_bridge_messages(context: &mut Context) { assert_eq!(*signature, signature_bytes); } } + +fn test_transfer_total_fails(context: &mut Context) { + // Initialize a wormhole bridge on Solana to test with. + let (ref payer, ref client, ref program) = common::setup(); + let emitter = Keypair::from_bytes(&GOVERNANCE_KEY).unwrap(); + let sequence = context.seq.next(emitter.pubkey().to_bytes()); + + // Be sure any previous tests have fully committed. + common::sync(client, payer); + + let fee_collector = FeeCollector::key(None, &program); + let account_balance = client.get_account(&fee_collector).unwrap().lamports; + + // Prepare to remove total balance, adding 10_000 to include the fee we're about to pay. + let recipient = Keypair::new(); + let nonce = rand::thread_rng().gen(); + let message = GovernancePayloadTransferFees { + amount: (account_balance + 10_000).into(), + to: payer.pubkey().to_bytes(), + } + .try_to_vec() + .unwrap(); + + let message_key = common::post_message( + client, + program, + payer, + &emitter, + nonce, + message.clone(), + 10_000, + false, + ) + .unwrap(); + + let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 1, 1); + common::verify_signatures(client, program, payer, body, body_hash, &context.secret, 1).unwrap(); + common::post_vaa(client, program, payer, vaa).unwrap(); + + // Transferring total fees should fail, to prevent the account being de-allocated. + assert!(common::transfer_fees( + client, + program, + payer, + message_key, + emitter.pubkey(), + payer.pubkey(), + sequence, + ) + .is_err()); + common::sync(client, payer); + + // The fee should have been paid, but other than that the balance should be exactly the same, + // I.E non-zero. + assert_eq!( + client.get_account(&fee_collector).unwrap().lamports, + account_balance + 10_000 + ); +}