Better program (#297)

* Add tests

* Add tests to CI

* Remote test from precommit hook

* New test

* Remove merge error

* Comments and increase seqno

* Remove unnecesary dep

* Fix rebase

* Fix feedback

* Move anchor checks

* Fix comment

* Restore deny warnings

* Better comments

* Typo

* Improve comments

* Better comment

* Tests for the remote executor (#298)

* Tests

* Cleanup

* Comments

* Comments

* Tests complete

Author: guibescos

pythnet/remote-executor/Cargo.lock

@@ -29,3 +29,4 @@ solana-program-test = "=1.10.31"
 tokio = "1.14.1"
 solana-sdk = "=1.10.31"
 bincode = "1.3.3"
+rand = "0.8.5"

pythnet/remote-executor/programs/remote-executor/Cargo.toml

@@ -6,17 +6,23 @@ use anchor_lang::{
     solana_program::borsh::get_packed_len,
     system_program,
 };
+use error::ExecutorError;
 use state::{
     claim_record::ClaimRecord,
     posted_vaa::AnchorVaa,
 };
+use wormhole::Chain::{
+    self,
+    Solana,
+};
 
 mod error;
 mod state;
 
-#[cfg(test)] //Conditional compilation of the tests
+#[cfg(test)]
 mod tests;
 
+//Anchor requires the program to declare its own id
 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");
 
 #[program]
@@ -25,32 +31,24 @@ pub mod remote_executor {
         instruction::Instruction,
         program::invoke_signed,
     };
-    use boolinator::Boolinator;
-    use wormhole::Chain::{
-        self,
-        Solana,
-    };
 
-    use crate::{
-        error::ExecutorError,
-        state::governance_payload::ExecutorPayload,
-    };
+    use crate::state::governance_payload::ExecutorPayload;
 
     use super::*;
 
     pub fn execute_posted_vaa(ctx: Context<ExecutePostedVaa>) -> Result<()> {
         let posted_vaa = &ctx.accounts.posted_vaa;
         let claim_record = &mut ctx.accounts.claim_record;
-
-        (Chain::from(posted_vaa.emitter_chain) == Solana)
-            .ok_or(error!(ExecutorError::EmitterChainNotSolana))?;
-        (posted_vaa.sequence > claim_record.sequence)
-            .ok_or(error!(ExecutorError::NonIncreasingSequence))?;
         claim_record.sequence = posted_vaa.sequence;
 
         let payload = ExecutorPayload::try_from_slice(&posted_vaa.payload)?;
         payload.check_header()?;
 
+        let (_, bump) = Pubkey::find_program_address(
+            &[EXECUTOR_KEY_SEED.as_bytes(), &posted_vaa.emitter_address],
+            &id(),
+        );
+
         for instruction in payload.instructions.iter().map(Instruction::from) {
             // TO DO: We currently pass `remaining_accounts` down to the CPIs, is there a more efficient way to do it?
             invoke_signed(
@@ -59,7 +57,7 @@ pub mod remote_executor {
                 &[&[
                     EXECUTOR_KEY_SEED.as_bytes(),
                     &posted_vaa.emitter_address,
-                    &[*ctx.bumps.get("executor_key").unwrap()],
+                    &[bump],
                 ]],
             )?;
         }
@@ -74,13 +72,16 @@ const CLAIM_RECORD_SEED: &str = "CLAIM_RECORD";
 pub struct ExecutePostedVaa<'info> {
     #[account(mut)]
     pub payer: Signer<'info>,
+    #[account(constraint = Chain::from(posted_vaa.emitter_chain) == Solana @ ExecutorError::EmitterChainNotSolana, constraint = posted_vaa.sequence > claim_record.sequence @ExecutorError::NonIncreasingSequence )]
     pub posted_vaa: Account<'info, AnchorVaa>,
-    #[account(seeds = [EXECUTOR_KEY_SEED.as_bytes(), &posted_vaa.emitter_address], bump)]
-    pub executor_key: UncheckedAccount<'info>,
-    /// The reason claim record is separated from executor_key is that executor key might need to pay in the CPI, so we want it to be a wallet
+    /// The reason claim_record has different seeds than executor_key is that executor key might need to pay in the CPI, so we want it to be a native wallet
     #[account(init_if_needed, space = 8 + get_packed_len::<ClaimRecord>(), payer=payer, seeds = [CLAIM_RECORD_SEED.as_bytes(), &posted_vaa.emitter_address], bump)]
     pub claim_record: Account<'info, ClaimRecord>,
     pub system_program: Program<'info, System>,
+    // Additional accounts passed to the instruction will be passed down to the CPIs. Very importantly executor_key needs to be passed as it will be the signer of the CPIs.
+    // Below is the "anchor specification" of that account
+    // #[account(seeds = [EXECUTOR_KEY_SEED.as_bytes(), &posted_vaa.emitter_address], bump)]
+    // pub executor_key: UncheckedAccount<'info>,
 }
 
 impl crate::accounts::ExecutePostedVaa {
@@ -90,19 +91,13 @@ impl crate::accounts::ExecutePostedVaa {
         emitter: &Pubkey,
         posted_vaa: &Pubkey,
     ) -> Self {
-        let executor_key = Pubkey::find_program_address(
-            &[EXECUTOR_KEY_SEED.as_bytes(), &emitter.to_bytes()],
-            program_id,
-        )
-        .0;
         let claim_record = Pubkey::find_program_address(
             &[CLAIM_RECORD_SEED.as_bytes(), &emitter.to_bytes()],
             program_id,
         )
         .0;
         crate::accounts::ExecutePostedVaa {
             payer: *payer,
-            executor_key,
             claim_record,
             posted_vaa: *posted_vaa,
             system_program: system_program::ID,