Format and lint executor (#287)

* Start anchor program

* pythnet folder

* Renames

* Implement processor

* Comments

* More comments

* Bump anchor

* Use new version of the wormhole package

* Get Solana chain id from wormhole core

* Pythnet bridge address

* Remove comment

* Fix borsh headers

* Format

* Fix precommit

* Fix precommit

* Precommit

* Add to CI

* Fix CI

Author: guibescos

.github/workflows/remote-executor-lints.yml

@@ -0,0 +1,19 @@
+name: Check Remote Executor
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: nightly
+          components: rustfmt, clippy
+      - uses: pre-commit/[email protected]

.pre-commit-config.yaml

@@ -0,0 +1,21 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v3.2.0
+    hooks:
+      - id: trailing-whitespace
+        files: pythnet/
+      - id: end-of-file-fixer
+        files: pythnet/
+      - id: check-added-large-files
+  - repo: local
+    hooks:
+      - id: cargo-fmt-executor
+        name: Cargo format executor
+        language: "rust"
+        entry: cargo +nightly fmt --manifest-path ./pythnet/remote-executor/Cargo.toml
+        pass_filenames: false
+      - id: cargo-clippy-executor
+        name: Cargo clippy executor
+        language: "rust"
+        entry: cargo +nightly clippy --manifest-path ./pythnet/remote-executor/Cargo.toml -- -D warnings
+        pass_filenames: false

pythnet/remote-executor/programs/remote-executor/src/error.rs

@@ -1,4 +1,3 @@
-
 use anchor_lang::prelude::*;
 
 #[error_code]
@@ -8,5 +7,5 @@ pub enum ExecutorError {
     GovernanceHeaderInvalidMagicNumber,
     GovernanceHeaderInvalidModule,
     GovernanceHeaderInvalidAction,
-    GovernanceHeaderInvalidReceiverChain
-}
+    GovernanceHeaderInvalidReceiverChain,
+}

pythnet/remote-executor/programs/remote-executor/src/lib.rs

@@ -1,61 +1,90 @@
 #![deny(warnings)]
+#![allow(clippy::result_large_err)]
 
-use anchor_lang::{prelude::*, solana_program::borsh::get_packed_len};
-use state::{claim_record::ClaimRecord, posted_vaa::AnchorVaa};
+use anchor_lang::{
+    prelude::*,
+    solana_program::borsh::get_packed_len,
+};
+use state::{
+    claim_record::ClaimRecord,
+    posted_vaa::AnchorVaa,
+};
 
-mod state;
 mod error;
+mod state;
 
 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");
 
 #[program]
 pub mod remote_executor {
-    use anchor_lang::solana_program::{program::invoke_signed, instruction::Instruction};
-    use wormhole::Chain::{Solana, self};
+    use anchor_lang::solana_program::{
+        instruction::Instruction,
+        program::invoke_signed,
+    };
+    use wormhole::Chain::{
+        self,
+        Solana,
+    };
 
-    use crate::{state::{governance_payload::ExecutorPayload}, error::ExecutorError};
+    use crate::{
+        error::ExecutorError,
+        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;
-        assert_or_err(Chain::from(posted_vaa.emitter_chain) == Solana, err!(ExecutorError::EmitterChainNotSolana))?;
-        assert_or_err(posted_vaa.sequence > claim_record.sequence, err!(ExecutorError::NonIncreasingSequence))?;
+        assert_or_err(
+            Chain::from(posted_vaa.emitter_chain) == Solana,
+            err!(ExecutorError::EmitterChainNotSolana),
+        )?;
+        assert_or_err(
+            posted_vaa.sequence > claim_record.sequence,
+            err!(ExecutorError::NonIncreasingSequence),
+        )?;
         claim_record.sequence = posted_vaa.sequence;
 
         let payload = ExecutorPayload::try_from_slice(&posted_vaa.payload)?;
         payload.check_header()?;
-        
+
         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(&instruction, ctx.remaining_accounts, &[&[EXECUTOR_KEY_SEED.as_bytes(), &posted_vaa.emitter_address, &[*ctx.bumps.get("executor_key").unwrap()]]])?;
+            invoke_signed(
+                &instruction,
+                ctx.remaining_accounts,
+                &[&[
+                    EXECUTOR_KEY_SEED.as_bytes(),
+                    &posted_vaa.emitter_address,
+                    &[*ctx.bumps.get("executor_key").unwrap()],
+                ]],
+            )?;
         }
         Ok(())
     }
 }
 
-const EXECUTOR_KEY_SEED : &str = "EXECUTOR_KEY";
-const CLAIM_RECORD_SEED : &str = "CLAIM_RECORD";
-
+const EXECUTOR_KEY_SEED: &str = "EXECUTOR_KEY";
+const CLAIM_RECORD_SEED: &str = "CLAIM_RECORD";
 
 #[derive(Accounts)]
 pub struct ExecutePostedVaa<'info> {
     #[account(mut)]
-    pub payer : Signer<'info>,
-    pub posted_vaa : Account<'info, AnchorVaa>,
+    pub payer: Signer<'info>,
+    pub posted_vaa: Account<'info, AnchorVaa>,
     #[account(seeds = [EXECUTOR_KEY_SEED.as_bytes(), &posted_vaa.emitter_address], bump)]
-    pub executor_key : UncheckedAccount<'info>,
+    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
     #[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>
+    pub claim_record: Account<'info, ClaimRecord>,
+    pub system_program: Program<'info, System>,
 }
 
-pub fn assert_or_err(condition : bool, error : Result<()>) -> Result<()>{
+pub fn assert_or_err(condition: bool, error: Result<()>) -> Result<()> {
     if !condition {
         error
     } else {
         Result::Ok(())
     }
-}
+}

pythnet/remote-executor/programs/remote-executor/src/state/claim_record.rs

@@ -1,10 +1,14 @@
-use anchor_lang::prelude::*;
-use anchor_lang::account;
-use anchor_lang::prelude::borsh::BorshSchema;
+use anchor_lang::{
+    account,
+    prelude::{
+        borsh::BorshSchema,
+        *,
+    },
+};
 
 #[account]
 #[derive(Default, BorshSchema)]
-/// This struct records 
-pub struct ClaimRecord{
-    pub sequence : u64 
-}
+/// This struct records
+pub struct ClaimRecord {
+    pub sequence: u64,
+}

pythnet/remote-executor/programs/remote-executor/src/state/governance_payload.rs

@@ -1,47 +1,56 @@
-use std::{ops::Deref, mem::size_of, io::ErrorKind};
-
-use anchor_lang::{prelude::*, solana_program::instruction::Instruction};
+use std::{
+    io::ErrorKind,
+    mem::size_of,
+    ops::Deref,
+};
+
+use anchor_lang::{
+    prelude::*,
+    solana_program::instruction::Instruction,
+};
 use wormhole::Chain;
 
-use crate::{assert_or_err, error::ExecutorError};
+use crate::{
+    assert_or_err,
+    error::ExecutorError,
+};
 
-pub const MAGIC_NUMBER : u32 = 0x4d475450; // Reverse order of the solidity contract because borsh uses little endian numbers
+pub const MAGIC_NUMBER: u32 = 0x4d475450; // Reverse order of the solidity contract because borsh uses little endian numbers
 
 #[derive(AnchorDeserialize, AnchorSerialize)]
-pub struct ExecutorPayload{
-    pub header : GovernanceHeader,
-    pub instructions: Vec<InstructionData>,
+pub struct ExecutorPayload {
+    pub header: GovernanceHeader,
 
+    pub instructions: Vec<InstructionData>,
 }
 
-#[derive(AnchorDeserialize, AnchorSerialize, PartialEq)]
+#[derive(AnchorDeserialize, AnchorSerialize, PartialEq, Eq)]
 pub enum Module {
     Executor = 0,
-    Target 
+    Target,
 }
 
-#[derive(AnchorDeserialize, AnchorSerialize, PartialEq)]
+#[derive(AnchorDeserialize, AnchorSerialize, PartialEq, Eq)]
 pub enum Action {
     ExecutePostedVaa = 0,
 }
 
-
 /// The Governance Header format for pyth governance messages is the following:
 /// - A 4 byte magic number `['P','T','G','M']`
 /// - A one byte module variant (0 for Executor and 1 for Target contracts)
 /// - A one byte action variant (for Executor only 0 is currently valid)
 /// - A bigendian 2 bytes u16 chain id
 #[derive(AnchorDeserialize, AnchorSerialize)]
 pub struct GovernanceHeader {
-    pub magic_number : u32,
-    pub module : Module,
-    pub action : Action,
-    pub chain : BigEndianU16
+    pub magic_number: u32,
+    pub module: Module,
+    pub action: Action,
+    pub chain: BigEndianU16,
 }
 
 /// Hack to get Borsh to deserialize, serialize this number with big endian order
-pub struct BigEndianU16{
-    pub value : u16
+pub struct BigEndianU16 {
+    pub value: u16,
 }
 
 impl AnchorDeserialize for BigEndianU16 {
@@ -54,7 +63,7 @@ impl AnchorDeserialize for BigEndianU16 {
         }
         let res = u16::from_be_bytes(buf[..size_of::<u16>()].try_into().unwrap());
         *buf = &buf[size_of::<u16>()..];
-        Ok(BigEndianU16{ value:res})
+        Ok(BigEndianU16 { value: res })
     }
 }
 
@@ -113,13 +122,25 @@ impl From<&InstructionData> for Instruction {
 }
 
 impl ExecutorPayload {
-    const MODULE : Module = Module::Executor;
-    const ACTION : Action = Action::ExecutePostedVaa;
-
-    pub fn check_header(&self) -> Result<()>{
-        assert_or_err(self.header.magic_number == MAGIC_NUMBER, err!(ExecutorError::GovernanceHeaderInvalidMagicNumber))?;
-        assert_or_err(self.header.module == ExecutorPayload::MODULE, err!(ExecutorError::GovernanceHeaderInvalidMagicNumber))?;
-        assert_or_err(self.header.action == ExecutorPayload::ACTION, err!(ExecutorError::GovernanceHeaderInvalidMagicNumber))?;
-        assert_or_err(Chain::from(self.header.chain.value) == Chain::Pythnet, err!(ExecutorError::GovernanceHeaderInvalidMagicNumber))
+    const MODULE: Module = Module::Executor;
+    const ACTION: Action = Action::ExecutePostedVaa;
+
+    pub fn check_header(&self) -> Result<()> {
+        assert_or_err(
+            self.header.magic_number == MAGIC_NUMBER,
+            err!(ExecutorError::GovernanceHeaderInvalidMagicNumber),
+        )?;
+        assert_or_err(
+            self.header.module == ExecutorPayload::MODULE,
+            err!(ExecutorError::GovernanceHeaderInvalidMagicNumber),
+        )?;
+        assert_or_err(
+            self.header.action == ExecutorPayload::ACTION,
+            err!(ExecutorError::GovernanceHeaderInvalidMagicNumber),
+        )?;
+        assert_or_err(
+            Chain::from(self.header.chain.value) == Chain::Pythnet,
+            err!(ExecutorError::GovernanceHeaderInvalidMagicNumber),
+        )
     }
-}
+}

pythnet/remote-executor/programs/remote-executor/src/state/mod.rs

@@ -1,3 +1,3 @@
 pub mod claim_record;
+pub mod governance_payload;
 pub mod posted_vaa;
-pub mod governance_payload;

pythnet/remote-executor/programs/remote-executor/src/state/posted_vaa.rs

@@ -1,10 +1,15 @@
-use std::{io::Write, str::FromStr, ops::Deref};
 use anchor_lang::prelude::*;
+use std::{
+    io::Write,
+    ops::Deref,
+    str::FromStr,
+};
 use wormhole_solana::VAA;
 
 impl Owner for AnchorVaa {
-    fn owner() -> Pubkey{
-        Pubkey::from_str("H3fxXJ86ADW2PNuDDmZJg6mzTtPxkYCpNuQUTgmJ7AjU").unwrap() // Pythnet bridge address
+    fn owner() -> Pubkey {
+        Pubkey::from_str("H3fxXJ86ADW2PNuDDmZJg6mzTtPxkYCpNuQUTgmJ7AjU").unwrap()
+        // Pythnet bridge address
     }
 }
 
@@ -15,16 +20,16 @@ impl AccountDeserialize for AnchorVaa {
     }
 
     // Manual implementation because this account does not have an anchor discriminator
-    fn try_deserialize_unchecked(buf: &mut &[u8]) -> Result<Self> {   
+    fn try_deserialize_unchecked(buf: &mut &[u8]) -> Result<Self> {
         AnchorDeserialize::deserialize(buf)
-        .map_err(|_| anchor_lang::error::ErrorCode::AccountDidNotDeserialize.into())
+            .map_err(|_| anchor_lang::error::ErrorCode::AccountDidNotDeserialize.into())
     }
 }
 
 impl AccountSerialize for AnchorVaa {
     // Make this fail, this is readonly VAA it should never be serialized by this program
     fn try_serialize<W: Write>(&self, _writer: &mut W) -> Result<()> {
-        return Err(anchor_lang::error::ErrorCode::AccountDidNotSerialize.into());
+        Err(anchor_lang::error::ErrorCode::AccountDidNotSerialize.into())
     }
 }
 
@@ -36,8 +41,7 @@ impl Deref for AnchorVaa {
     }
 }
 
-
 #[derive(Clone, AnchorDeserialize, AnchorSerialize)]
-pub struct AnchorVaa{
-    pub vaa : VAA
+pub struct AnchorVaa {
+    pub vaa: VAA,
 }

pythnet/remote-executor/rustfmt.toml

@@ -0,0 +1,11 @@
+# Merge similar crates together to avoid multiple use statements.
+imports_granularity = "Crate"
+
+# Consistency in formatting makes tool based searching/editing better.
+empty_item_single_line = false
+
+# Easier editing when arbitrary mixed use statements do not collapse.
+imports_layout = "Vertical"
+
+# Default rustfmt formatting of match arms with branches is awful.
+match_arm_leading_pipes = "Preserve"