Skip to content

Commit

Permalink
pr comments 2
Browse files Browse the repository at this point in the history
- add InvalidChainId for when chain_id = 0 is passed in
- check if integrator_chain_config is initialized by checking
  integrator_chain_config.chain_id == 0
- remove redundant is_some() check
- remove redundant update_admin function

Signed-off-by: bingyuyap <[email protected]>
  • Loading branch information
bingyuyap committed Oct 24, 2024
1 parent 54e887b commit 2a6d8da
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 20 deletions.
58 changes: 53 additions & 5 deletions svm/programs/mock-integrator/tests/enable_transceiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ async fn verify_transceiver_state(
integrator_chain_config_pda: Pubkey,
expected_recv_bitmap: u128,
expected_send_bitmap: u128,
expected_chain_id: u16,
expected_integrator_id: Pubkey,
) {
let integrator_chain_config: IntegratorChainConfig =
get_account(&mut context.banks_client, integrator_chain_config_pda).await;
Expand All @@ -91,6 +93,11 @@ async fn verify_transceiver_state(
integrator_chain_config.send_transceiver_bitmap,
Bitmap::from_value(expected_send_bitmap)
);
assert_eq!(integrator_chain_config.chain_id, expected_chain_id);
assert_eq!(
integrator_chain_config.integrator_program_id,
expected_integrator_id
);
}

#[tokio::test]
Expand Down Expand Up @@ -122,7 +129,15 @@ async fn test_enable_in_transceivers_success() {
.await;
assert!(result.is_ok());

verify_transceiver_state(&mut context, integrator_chain_config_pda, 1, 0).await;
verify_transceiver_state(
&mut context,
integrator_chain_config_pda,
1,
0,
chain_id,
integrator_program_id,
)
.await;
}

#[tokio::test]
Expand Down Expand Up @@ -187,7 +202,16 @@ async fn test_enable_in_transceivers_multiple_sets_success() {
assert!(result.is_ok());

// Verify that both transceivers are set
verify_transceiver_state(&mut context, integrator_chain_config_pda, 0b11, 0).await;

verify_transceiver_state(
&mut context,
integrator_chain_config_pda,
3,
0,
chain_id,
integrator_program_id,
)
.await;
}

#[tokio::test]
Expand Down Expand Up @@ -220,7 +244,15 @@ async fn test_enable_out_transceivers_success() {

assert!(result.is_ok());

verify_transceiver_state(&mut context, integrator_chain_config_pda, 0, 1).await;
verify_transceiver_state(
&mut context,
integrator_chain_config_pda,
0,
1,
chain_id,
integrator_program_id,
)
.await;
}

#[tokio::test]
Expand Down Expand Up @@ -335,7 +367,15 @@ async fn test_enable_already_enabled_transceiver() {
.await;
assert!(result.is_ok());

verify_transceiver_state(&mut context, integrator_chain_config_pda, 1, 0).await;
verify_transceiver_state(
&mut context,
integrator_chain_config_pda,
1,
0,
chain_id,
integrator_program_id,
)
.await;

// Second attempt: should fail with TransceiverAlreadyEnabled
let result = enable_recv_transceiver(
Expand All @@ -362,7 +402,15 @@ async fn test_enable_already_enabled_transceiver() {
);

// Verify that the state hasn't changed
verify_transceiver_state(&mut context, integrator_chain_config_pda, 1, 0).await;
verify_transceiver_state(
&mut context,
integrator_chain_config_pda,
1,
0,
chain_id,
integrator_program_id,
)
.await;
}

#[tokio::test]
Expand Down
3 changes: 3 additions & 0 deletions svm/programs/router/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,7 @@ pub enum RouterError {

#[msg("No admin transfer is in progress")]
NoAdminTransferInProgress,

#[msg("Invalid Chain Id")]
InvalidChainId,
}
40 changes: 34 additions & 6 deletions svm/programs/router/src/instructions/enable_transceiver.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::error::RouterError;
use crate::instructions::common::TransceiverInfoArgs;
use crate::state::{IntegratorChainConfig, IntegratorConfig, TransceiverInfo};
use crate::utils::bitmap::Bitmap;
use anchor_lang::prelude::*;

#[derive(Accounts)]
Expand Down Expand Up @@ -56,8 +57,13 @@ pub struct EnableTransceiver<'info> {
}

impl<'info> EnableTransceiver<'info> {
pub fn validate(&self) -> Result<()> {
self.integrator_config.check_admin(&self.admin)
pub fn validate(&self, args: &TransceiverInfoArgs) -> Result<()> {
self.integrator_config.check_admin(&self.admin)?;

// Ensure chain_id is not zero
require!(args.chain_id != 0, RouterError::InvalidChainId);

Ok(())
}
}

Expand All @@ -74,14 +80,25 @@ impl<'info> EnableTransceiver<'info> {
/// # Returns
///
/// * `Result<()>` - The result of the operation
#[access_control(EnableTransceiver::validate(&ctx.accounts))]
#[access_control(EnableTransceiver::validate(&ctx.accounts, &args))]
pub fn enable_recv_transceiver(
ctx: Context<EnableTransceiver>,
_args: TransceiverInfoArgs,
args: TransceiverInfoArgs,
) -> Result<()> {
let registered_transceiver = &ctx.accounts.registered_transceiver;
let integrator_chain_config = &mut ctx.accounts.integrator_chain_config;

// If chain_id is 0, this is initial setup
if integrator_chain_config.chain_id == 0 {
integrator_chain_config.set_inner(IntegratorChainConfig {
chain_id: args.chain_id,
bump: ctx.bumps.integrator_chain_config,
integrator_program_id: args.integrator_program_id,
send_transceiver_bitmap: Bitmap::new(),
recv_transceiver_bitmap: Bitmap::new(),
});
}

if integrator_chain_config
.recv_transceiver_bitmap
.get(registered_transceiver.index)?
Expand Down Expand Up @@ -109,14 +126,25 @@ pub fn enable_recv_transceiver(
/// # Returns
///
/// * `Result<()>` - The result of the operation
#[access_control(EnableTransceiver::validate(&ctx.accounts))]
#[access_control(EnableTransceiver::validate(&ctx.accounts, &args))]
pub fn enable_send_transceiver(
ctx: Context<EnableTransceiver>,
_args: TransceiverInfoArgs,
args: TransceiverInfoArgs,
) -> Result<()> {
let registered_transceiver = &ctx.accounts.registered_transceiver;
let integrator_chain_config = &mut ctx.accounts.integrator_chain_config;

// If chain_id is 0, this is initial setup
if integrator_chain_config.chain_id == 0 {
integrator_chain_config.set_inner(IntegratorChainConfig {
chain_id: args.chain_id,
bump: ctx.bumps.integrator_chain_config,
integrator_program_id: args.integrator_program_id,
send_transceiver_bitmap: Bitmap::new(),
recv_transceiver_bitmap: Bitmap::new(),
});
}

if integrator_chain_config
.send_transceiver_bitmap
.get(registered_transceiver.index)?
Expand Down
4 changes: 1 addition & 3 deletions svm/programs/router/src/instructions/update_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ pub fn update_admin(ctx: Context<UpdateAdmin>, args: UpdateAdminArgs) -> Result<
return Err(RouterError::AdminTransferInProgress.into());
}

ctx.accounts
.integrator_config
.update_admin(args.new_admin)?;
ctx.accounts.integrator_config.admin = Some(args.new_admin);

Ok(())
}
7 changes: 1 addition & 6 deletions svm/programs/router/src/state/integrator_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl IntegratorConfig {

pub fn check_admin(&self, signer: &Signer) -> Result<()> {
require!(
self.admin.is_some() && self.admin == Some(signer.key()),
self.admin == Some(signer.key()),
RouterError::CallerNotAuthorized
);
require!(
Expand All @@ -53,11 +53,6 @@ impl IntegratorConfig {
Ok(())
}

pub fn update_admin(&mut self, new_admin: Pubkey) -> Result<()> {
self.admin = Some(new_admin);
Ok(())
}

/// The `init` constraint in the add_transceiver instruction checks that the transceiver has not been added. If it is,
/// `AccountAlreadyInUse` error will be thrown
pub fn add_transceiver(&mut self, transceiver: Pubkey) -> Result<()> {
Expand Down

0 comments on commit 2a6d8da

Please sign in to comment.