Address review comments

Change-Id: I4036e71f3df1dc419419930cfdffba302166e28d

solana/bridge/program/src/api/post_message.rs

@@ -14,9 +14,7 @@ use crate::{
     types::ConsistencyLevel,
     CHAIN_ID_SOLANA,
 };
-use solana_program::{
+use solana_program::sysvar::clock::Clock;
-    sysvar::clock::Clock,
-};
 use solitaire::{
     processors::seeded::Seeded,
     trace,
@@ -81,7 +79,8 @@ pub fn post_message(
     trace!("Emitter Address: {}", accs.emitter.info().key);
     trace!("Nonce: {}", data.nonce);
 
-    accs.sequence.verify_derivation(ctx.program_id, &(&*accs).into())?;
+    accs.sequence
+        .verify_derivation(ctx.program_id, &(&*accs).into())?;
 
     let msg_derivation = MessageDerivationData {
         emitter_key: accs.emitter.key.to_bytes(),
@@ -121,7 +120,7 @@ pub fn post_message(
             .create(&(&*accs).into(), ctx, accs.payer.key, Exempt)?;
     }
 
-	trace!("Sequence: {}", accs.sequence.sequence);
+    trace!("Sequence: {}", accs.sequence.sequence);
 
     // Initialize transfer
     trace!("Setting Message Details");

solana/bridge/program/src/api/post_vaa.rs

@@ -29,6 +29,10 @@ use byteorder::{
     BigEndian,
     WriteBytesExt,
 };
+use serde::{
+    Deserialize,
+    Serialize,
+};
 use sha3::Digest;
 use solana_program::program_error::ProgramError;
 use solitaire::{
@@ -39,10 +43,6 @@ use std::io::{
     Cursor,
     Write,
 };
-use serde::{
-    Deserialize,
-    Serialize
-};
 
 impl From<&PostVAAData> for GuardianSetDerivationData {
     fn from(data: &PostVAAData) -> Self {

solana/bridge/program/src/types.rs

@@ -16,7 +16,14 @@ use byteorder::{
     ReadBytesExt,
 };
 use primitive_types::U256;
-use solana_program::pubkey::Pubkey;
+use serde::{
+    Deserialize,
+    Serialize,
+};
+use solana_program::{
+    program_error::ProgramError::InvalidAccountData,
+    pubkey::Pubkey,
+};
 use solitaire::{
     processors::seeded::{
         AccountOwner,
@@ -36,10 +43,6 @@ use std::{
     },
     str::FromStr,
 };
-use serde::{
-    Deserialize,
-    Serialize,
-};
 
 #[derive(Default, BorshSerialize, BorshDeserialize, Serialize, Deserialize)]
 pub struct GuardianSetData {
@@ -237,8 +240,8 @@ impl SerializePayload for GovernancePayloadUpgrade {
 }
 
 impl DeserializePayload for GovernancePayloadUpgrade
-    where
+where
-        Self: DeserializeGovernancePayload,
+    Self: DeserializeGovernancePayload,
 {
     fn deserialize(buf: &mut &[u8]) -> Result<Self, SolitaireError> {
         let mut c = Cursor::new(buf);
@@ -247,6 +250,10 @@ impl DeserializePayload for GovernancePayloadUpgrade
         let mut addr = [0u8; 32];
         c.read_exact(&mut addr)?;
 
+        if c.position() != c.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(GovernancePayloadUpgrade {
             new_contract: Pubkey::new(&addr[..]),
         })
@@ -282,8 +289,8 @@ impl SerializePayload for GovernancePayloadGuardianSetChange {
 }
 
 impl DeserializePayload for GovernancePayloadGuardianSetChange
-    where
+where
-        Self: DeserializeGovernancePayload,
+    Self: DeserializeGovernancePayload,
 {
     fn deserialize(buf: &mut &[u8]) -> Result<Self, SolitaireError> {
         let mut c = Cursor::new(buf);
@@ -299,6 +306,10 @@ impl DeserializePayload for GovernancePayloadGuardianSetChange
             keys.push(key);
         }
 
+        if c.position() != c.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(GovernancePayloadGuardianSetChange {
             new_guardian_set_index: new_index,
             new_guardian_set: keys,
@@ -330,8 +341,8 @@ impl SerializePayload for GovernancePayloadSetMessageFee {
 }
 
 impl DeserializePayload for GovernancePayloadSetMessageFee
-    where
+where
-        Self: DeserializeGovernancePayload,
+    Self: DeserializeGovernancePayload,
 {
     fn deserialize(buf: &mut &[u8]) -> Result<Self, SolitaireError> {
         let mut c = Cursor::new(buf);
@@ -341,6 +352,10 @@ impl DeserializePayload for GovernancePayloadSetMessageFee
         c.read_exact(&mut fee_data)?;
         let fee = U256::from_big_endian(&fee_data);
 
+        if c.position() != c.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(GovernancePayloadSetMessageFee { fee })
     }
 }
@@ -372,8 +387,8 @@ impl SerializePayload for GovernancePayloadTransferFees {
 }
 
 impl DeserializePayload for GovernancePayloadTransferFees
-    where
+where
-        Self: DeserializeGovernancePayload,
+    Self: DeserializeGovernancePayload,
 {
     fn deserialize(buf: &mut &[u8]) -> Result<Self, SolitaireError> {
         let mut c = Cursor::new(buf);
@@ -386,6 +401,10 @@ impl DeserializePayload for GovernancePayloadTransferFees
         let mut to = ForeignAddress::default();
         c.read_exact(&mut to)?;
 
+        if c.position() != c.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(GovernancePayloadTransferFees { amount, to })
     }
 }

solana/bridge/program/src/vaa.rs

@@ -3,6 +3,7 @@ use crate::{
         Claim,
         ClaimDerivationData,
     },
+    api::ForeignAddress,
     error::Error::{
         InvalidGovernanceAction,
         InvalidGovernanceChain,
@@ -17,6 +18,10 @@ use byteorder::{
     BigEndian,
     ReadBytesExt,
 };
+use serde::{
+    Deserialize,
+    Serialize,
+};
 use solana_program::pubkey::Pubkey;
 use solitaire::{
     processors::seeded::Seeded,
@@ -37,8 +42,6 @@ use std::{
     },
     ops::Deref,
 };
-use crate::api::ForeignAddress;
-use serde::{Deserialize, Serialize};
 
 pub trait SerializePayload: Sized {
     fn serialize<W: Write>(&self, writer: &mut W) -> std::result::Result<(), SolitaireError>;
@@ -110,8 +113,8 @@ pub struct PayloadMessage<'b, T: DeserializePayload>(
 
 impl<'a, 'b: 'a, 'c, T: DeserializePayload> Peel<'a, 'b, 'c> for PayloadMessage<'b, T> {
     fn peel<I>(ctx: &'c mut Context<'a, 'b, 'c, I>) -> Result<Self>
-        where
+    where
-            Self: Sized,
+        Self: Sized,
     {
         // Deserialize wrapped payload
         let data: Data<'b, PostedMessage, { AccountState::Initialized }> = Data::peel(ctx)?;

solana/modules/token_bridge/program/src/api/complete_transfer.rs

@@ -14,7 +14,10 @@ use crate::{
     types::*,
     TokenBridgeError::*,
 };
-use bridge::vaa::ClaimableVAA;
+use bridge::{
+    vaa::ClaimableVAA,
+    CHAIN_ID_SOLANA,
+};
 use solana_program::{
     account_info::AccountInfo,
     program::invoke_signed,
@@ -83,11 +86,13 @@ pub fn complete_native(
 ) -> Result<()> {
     // Verify the chain registration
     let derivation_data: EndpointDerivationData = (&*accs).into();
-    accs.chain_registration.verify_derivation(ctx.program_id, &derivation_data)?;
+    accs.chain_registration
+        .verify_derivation(ctx.program_id, &derivation_data)?;
 
     // Verify that the custody account is derived correctly
     let derivation_data: CustodyAccountDerivationData = (&*accs).into();
-    accs.custody.verify_derivation(ctx.program_id, &derivation_data)?;
+    accs.custody
+        .verify_derivation(ctx.program_id, &derivation_data)?;
 
     // Verify mints
     if *accs.mint.info().key != accs.to.mint {
@@ -107,6 +112,9 @@ pub fn complete_native(
     if accs.vaa.token_chain != 1 {
         return Err(InvalidChain.into());
     }
+    if accs.vaa.to_chain != CHAIN_ID_SOLANA {
+        return Err(InvalidChain.into());
+    }
 
     // Prevent vaa double signing
     accs.vaa.verify(ctx.program_id)?;
@@ -187,13 +195,19 @@ pub fn complete_wrapped(
 
     // Verify mint
     let derivation_data: WrappedDerivationData = (&*accs).into();
-    accs.mint.verify_derivation(ctx.program_id, &derivation_data)?;
+    accs.mint
+        .verify_derivation(ctx.program_id, &derivation_data)?;
 
     // Verify mints
     if *accs.mint.info().key != accs.to.mint {
         return Err(InvalidMint.into());
     }
 
+    // Verify VAA
+    if accs.vaa.to_chain != CHAIN_ID_SOLANA {
+        return Err(InvalidChain.into());
+    }
+
     accs.vaa.verify(ctx.program_id)?;
     accs.vaa.claim(ctx, accs.payer.key)?;
 

solana/modules/token_bridge/program/src/api/create_wrapped.rs

@@ -4,8 +4,8 @@ use crate::{
         Endpoint,
         EndpointDerivationData,
         MintSigner,
-        WrappedMetaDerivationData,
         WrappedDerivationData,
+        WrappedMetaDerivationData,
         WrappedMint,
         WrappedTokenMeta,
     },
@@ -92,13 +92,16 @@ pub fn create_wrapped(
     data: CreateWrappedData,
 ) -> Result<()> {
     let derivation_data: WrappedDerivationData = (&*accs).into();
-    accs.mint.verify_derivation(ctx.program_id, &derivation_data)?;
+    accs.mint
+        .verify_derivation(ctx.program_id, &derivation_data)?;
 
     let meta_derivation_data: WrappedMetaDerivationData = (&*accs).into();
-    accs.meta.verify_derivation(ctx.program_id, &meta_derivation_data)?;
+    accs.meta
+        .verify_derivation(ctx.program_id, &meta_derivation_data)?;
 
     let derivation_data: EndpointDerivationData = (&*accs).into();
-    accs.chain_registration.verify_derivation(ctx.program_id, &derivation_data)?;
+    accs.chain_registration
+        .verify_derivation(ctx.program_id, &derivation_data)?;
 
     accs.vaa.verify(ctx.program_id)?;
     accs.vaa.claim(ctx, accs.payer.key)?;

solana/modules/token_bridge/program/src/api/governance.rs

@@ -9,7 +9,10 @@ use crate::{
         PayloadGovernanceRegisterChain,
     },
     types::*,
-    TokenBridgeError::InvalidGovernanceKey,
+    TokenBridgeError::{
+        InvalidChain,
+        InvalidGovernanceKey,
+    },
 };
 use bridge::{
     vaa::{
@@ -151,13 +154,18 @@ pub fn register_chain(
     data: RegisterChainData,
 ) -> Result<()> {
     let derivation_data: EndpointDerivationData = (&*accs).into();
-    accs.endpoint.verify_derivation(ctx.program_id, &derivation_data)?;
+    accs.endpoint
+        .verify_derivation(ctx.program_id, &derivation_data)?;
 
     // Claim VAA
     verify_governance(&accs.vaa);
     accs.vaa.verify(&ctx.program_id)?;
     accs.vaa.claim(ctx, accs.payer.key)?;
 
+    if accs.vaa.chain == CHAIN_ID_SOLANA {
+        return Err(InvalidChain.into());
+    }
+
     // Create endpoint
     accs.endpoint
         .create(&((&*accs).into()), ctx, accs.payer.key, Exempt);

solana/modules/token_bridge/program/src/api/transfer.rs

@@ -282,10 +282,8 @@ pub fn transfer_wrapped(
 
     // Verify that meta is correct
     let derivation_data: WrappedMetaDerivationData = (&*accs).into();
-    accs.wrapped_meta.verify_derivation(
+    accs.wrapped_meta
-        ctx.program_id,
+        .verify_derivation(ctx.program_id, &derivation_data)?;
-        &derivation_data,
-    )?;
 
     // Burn tokens
     let burn_ix = spl_token::instruction::burn(

solana/modules/token_bridge/program/src/instructions.rs

@@ -216,9 +216,7 @@ pub fn create_wrapped(
         &program_id,
     );
     let mint_meta_key = WrappedTokenMeta::<'_, { AccountState::Uninitialized }>::key(
-        &WrappedMetaDerivationData {
+        &WrappedMetaDerivationData { mint_key },
-            mint_key,
-        },
         &program_id,
     );
     let mint_authority_key = MintSigner::key(None, &program_id);

solana/modules/token_bridge/program/src/messages.rs

@@ -25,7 +25,10 @@ use byteorder::{
 use primitive_types::U256;
 use solana_program::{
     native_token::Sol,
-    program_error::ProgramError,
+    program_error::{
+        ProgramError,
+        ProgramError::InvalidAccountData,
+    },
     pubkey::Pubkey,
 };
 use solitaire::SolitaireError;
@@ -82,6 +85,10 @@ impl DeserializePayload for PayloadTransfer {
         v.read_exact(&mut fee_data)?;
         let fee = U256::from_big_endian(&fee_data);
 
+        if v.position() != v.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(PayloadTransfer {
             amount,
             token_address,
@@ -155,6 +162,10 @@ impl DeserializePayload for PayloadAssetMeta {
             .map_err::<SolitaireError, _>(|_| TokenBridgeError::InvalidUTF8String.into())?;
         name = name.chars().filter(|c| c != &'\0').collect();
 
+        if v.position() != v.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(PayloadAssetMeta {
             token_address,
             token_chain,
@@ -219,6 +230,10 @@ where
         let mut endpoint_address = [0u8; 32];
         v.read_exact(&mut endpoint_address)?;
 
+        if v.position() != v.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(PayloadGovernanceRegisterChain {
             chain,
             endpoint_address,
@@ -265,6 +280,10 @@ where
         let mut addr = [0u8; 32];
         c.read_exact(&mut addr)?;
 
+        if c.position() != c.into_inner().len() as u64 {
+            return Err(InvalidAccountData.into());
+        }
+
         Ok(GovernancePayloadUpgrade {
             new_contract: Pubkey::new(&addr[..]),
         })