Skip to content

Commit

Permalink
fix: audit feedback 1 (#1331)
Browse files Browse the repository at this point in the history
* Audit 1

* Pin wh sdk

* Typo

* Typo

* Add constraint to initialize
  • Loading branch information
guibescos authored Mar 1, 2024
1 parent 736a202 commit 7bf2782
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,6 @@ pub enum ReceiverError {
TargetGovernanceAuthorityMismatch,
#[msg("The governance authority needs to request a transfer first")]
NonexistentGovernanceAuthorityTransferRequest,
#[msg("The minimum number of signatures should be at least 1")]
ZeroMinimumSignatures,
}
20 changes: 15 additions & 5 deletions target_chains/solana/programs/pyth-solana-receiver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ pub mod pyth_solana_receiver {
use super::*;

pub fn initialize(ctx: Context<Initialize>, initial_config: Config) -> Result<()> {
require!(
initial_config.minimum_signatures > 0,
ReceiverError::ZeroMinimumSignatures
);
let config = &mut ctx.accounts.config;
**config = initial_config;
Ok(())
Expand Down Expand Up @@ -101,14 +105,21 @@ pub mod pyth_solana_receiver {

pub fn set_minimum_signatures(ctx: Context<Governance>, minimum_signatures: u8) -> Result<()> {
let config = &mut ctx.accounts.config;
require!(minimum_signatures > 0, ReceiverError::ZeroMinimumSignatures);
config.minimum_signatures = minimum_signatures;
Ok(())
}

/// Post a price update using a VAA and a MerklePriceUpdate.
/// This function allows you to post a price update in a single transaction.
/// Compared to post_update, it is less secure since you won't be able to verify all guardian signatures if you use this function because of transaction size limitations.
/// Typically, you can fit 5 guardian signatures in a transaction that uses this.
/// Compared to `post_update`, it only checks whatever signatures are present in the provided VAA and doesn't fail if the number of signatures is lower than the Wormhole quorum of two thirds of the guardians.
/// The number of signatures that were in the VAA is stored in the `VerificationLevel` of the `PriceUpdateV1` account.
///
/// We recommend using `post_update_atomic` with 5 signatures. This is close to the maximum signatures you can verify in one transaction without exceeding the transaction size limit.
///
/// # Warning
///
/// Using partially verified price updates is dangerous, as it lowers the threshold of guardians that need to collude to produce a malicious price update.
pub fn post_update_atomic(
ctx: Context<PostUpdateAtomic>,
params: PostUpdateAtomicParams,
Expand Down Expand Up @@ -195,7 +206,7 @@ pub mod pyth_solana_receiver {
pub fn post_update(ctx: Context<PostUpdate>, params: PostUpdateParams) -> Result<()> {
let config = &ctx.accounts.config;
let payer: &Signer<'_> = &ctx.accounts.payer;
let encoded_vaa = VaaAccount::load(&ctx.accounts.encoded_vaa)?;
let encoded_vaa = VaaAccount::load(&ctx.accounts.encoded_vaa)?; // IMPORTANT: This line checks that the encoded_vaa has ProcessingStatus::Verified. This check is critical otherwise the program could be tricked into accepting unverified VAAs.
let treasury: &AccountInfo<'_> = &ctx.accounts.treasury;
let price_update_account: &mut Account<'_, PriceUpdateV1> =
&mut ctx.accounts.price_update_account;
Expand Down Expand Up @@ -269,9 +280,8 @@ pub struct PostUpdate<'info> {
pub encoded_vaa: AccountInfo<'info>,
#[account(seeds = [CONFIG_SEED.as_ref()], bump)]
pub config: Account<'info, Config>,
#[account(seeds = [TREASURY_SEED.as_ref(), &[params.treasury_id]], bump)]
/// CHECK: This is just a PDA controlled by the program. There is currently no way to withdraw funds from it.
#[account(mut)]
#[account(mut, seeds = [TREASURY_SEED.as_ref(), &[params.treasury_id]], bump)]
pub treasury: AccountInfo<'info>,
/// The contraint is such that either the price_update_account is uninitialized or the payer is the write_authority.
/// Pubkey::default() is the SystemProgram on Solana and it can't sign so it's impossible that price_update_account.write_authority == Pubkey::default() once the account is initialized
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,20 @@ async fn test_governance() {
initial_config.minimum_signatures
);

// Minimum signatures can't be 0
assert_eq!(
program_simulator
.process_ix_with_default_compute_limit(
SetMinimumSignatures::populate(governance_authority.pubkey(), 0,),
&vec![&governance_authority],
None,
)
.await
.unwrap_err()
.unwrap(),
into_transaction_error(ReceiverError::ZeroMinimumSignatures)
);

program_simulator
.process_ix_with_default_compute_limit(
SetMinimumSignatures::populate(
Expand Down

0 comments on commit 7bf2782

Please sign in to comment.