Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(svm): root bundles over cctp #687

Merged
merged 10 commits into from
Oct 25, 2024
2 changes: 2 additions & 0 deletions programs/svm-spoke/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ pub enum CalldataError {
InvalidBool,
#[msg("Invalid solidity address argument")]
InvalidAddress,
#[msg("Invalid solidity uint32 argument")]
InvalidUint32,
#[msg("Invalid solidity uint64 argument")]
InvalidUint64,
#[msg("Invalid solidity uint128 argument")]
Expand Down
14 changes: 10 additions & 4 deletions programs/svm-spoke/src/instructions/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,20 @@ pub fn set_enable_route(
#[derive(Accounts)]
pub struct RelayRootBundle<'info> {
#[account(
mut, // TODO: remove this mut and have separate payer when adding support to invoke this via CCTP.
constraint = is_local_or_remote_owner(&signer, &state) @ CustomError::NotOwner
)]
pub signer: Signer<'info>,

#[account(mut)]
pub payer: Signer<'info>,

// TODO: standardize usage of state.seed vs state.key()
#[account(mut, seeds = [b"state", state.seed.to_le_bytes().as_ref()], bump)]
pub state: Account<'info, State>,

// TODO: consider deriving seed from state.seed instead of state.key() as this could be cheaper (need to verify).
#[account(init, // TODO: add comment explaining why init
payer = signer,
payer = payer,
space = DISCRIMINATOR_SIZE + RootBundle::INIT_SPACE,
seeds =[b"root_bundle", state.key().as_ref(), state.root_bundle_id.to_le_bytes().as_ref()],
bump)]
Expand Down Expand Up @@ -257,17 +259,21 @@ pub fn relay_root_bundle(
#[instruction(root_bundle_id: u32)]
pub struct EmergencyDeleteRootBundle<'info> {
#[account(
mut,
constraint = is_local_or_remote_owner(&signer, &state) @ CustomError::NotOwner
)]
pub signer: Signer<'info>,

#[account(mut)]
// We do not restrict who can receive lamports from closing root_bundle account as that would require storing the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, as long as it's authenticated with is_local_or_remote_owner we don't really mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In remote authentication this checks only that this is coming from HubPool via CCTP, but we do not use permissioned caller to receive the message on Solana side (though CCTP has that option). So there might be a case that someone frontruns receiving message on Solana and gets all lamports from the closing of root bundle account. But I think this is rare enough when we would use this emergency method and it should be no concern that someone else could gain some extra lamports.

Copy link
Contributor

@md0x md0x Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Anyone can “finalize” these messages.

However, I’m wondering if it might make sense to use the feature you mentioned, where CCTP allows authentication of specific finalizers, so we can handle these admin actions with more control if needed. (Just throwing an open question)

// original payer when root bundle was relayed and unnecessarily make it more expensive to relay in the happy path.
pub closer: SystemAccount<'info>,

#[account(seeds = [b"state", state.seed.to_le_bytes().as_ref()], bump)]
pub state: Account<'info, State>,

#[account(mut,
seeds =[b"root_bundle", state.key().as_ref(), root_bundle_id.to_le_bytes().as_ref()],
close = signer,
close = closer,
bump)]
pub root_bundle: Account<'info, RootBundle>,
}
Expand Down
12 changes: 12 additions & 0 deletions programs/svm-spoke/src/instructions/handle_receive_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ fn translate_message(data: &Vec<u8>) -> Result<Vec<u8>> {
(origin_token, destination_chain_id, enabled)
.encode_instruction_data("global:set_enable_route")
}
s if s == utils::encode_solidity_selector("relayRootBundle(bytes32,bytes32)") => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great! we can now test this on testnet as well over cctp

let relayer_refund_root = utils::get_solidity_arg(data, 0)?;
let slow_relay_root = utils::get_solidity_arg(data, 1)?;

(relayer_refund_root, slow_relay_root)
.encode_instruction_data("global:relay_root_bundle")
}
s if s == utils::encode_solidity_selector("emergencyDeleteRootBundle(uint256)") => {
let root_id = utils::decode_solidity_uint32(&utils::get_solidity_arg(data, 0)?)?;

root_id.encode_instruction_data("global:emergency_delete_root_bundle")
}
_ => Err(CalldataError::UnsupportedSelector.into()),
}
}
Expand Down
9 changes: 9 additions & 0 deletions programs/svm-spoke/src/utils/cctp_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ pub fn get_self_authority_pda() -> Pubkey {
pda_address
}

pub fn decode_solidity_uint32(data: &[u8; 32]) -> Result<u32> {
let h_value = u128::from_be_bytes(data[..16].try_into().unwrap());
let l_value = u128::from_be_bytes(data[16..].try_into().unwrap());
if h_value > 0 || l_value > u32::MAX as u128 {
return err!(CalldataError::InvalidUint32);
}
Ok(l_value as u32)
}

pub fn decode_solidity_uint64(data: &[u8; 32]) -> Result<u64> {
let h_value = u128::from_be_bytes(data[..16].try_into().unwrap());
let l_value = u128::from_be_bytes(data[16..].try_into().unwrap());
Expand Down
1 change: 1 addition & 0 deletions scripts/svm/simpleFakeRelayerRepayment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ async function testBundleLogic(): Promise<void> {
state: statePda,
rootBundle: rootBundle,
signer: signer,
payer: signer,
systemProgram: SystemProgram.programId,
})
.rpc();
Expand Down
59 changes: 42 additions & 17 deletions test/svm/SvmSpoke.Bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,13 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Try to relay root bundle as non-owner
let relayRootBundleAccounts = { state: state, rootBundle, signer: nonOwner.publicKey, program: program.programId };
let relayRootBundleAccounts = {
state: state,
rootBundle,
signer: nonOwner.publicKey,
payer: nonOwner.publicKey,
program: program.programId,
};
try {
await program.methods
.relayRootBundle(relayerRefundRootArray, slowRelayRootArray)
Expand All @@ -114,7 +120,7 @@ describe("svm_spoke.bundle", () => {
}

// Relay root bundle as owner
relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods
.relayRootBundle(relayerRefundRootArray, slowRelayRootArray)
.accounts(relayRootBundleAccounts)
Expand Down Expand Up @@ -146,7 +152,13 @@ describe("svm_spoke.bundle", () => {
const seeds2 = [Buffer.from("root_bundle"), state.toBuffer(), rootBundleIdBuffer2];
const [rootBundle2] = PublicKey.findProgramAddressSync(seeds2, program.programId);

relayRootBundleAccounts = { state, rootBundle: rootBundle2, signer: owner, program: program.programId };
relayRootBundleAccounts = {
state,
rootBundle: rootBundle2,
signer: owner,
payer: owner,
program: program.programId,
};
await program.methods
.relayRootBundle(relayerRefundRootArray2, slowRelayRootArray2)
.accounts(relayRootBundleAccounts)
Expand All @@ -170,7 +182,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle as owner
const relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
const tx = await program.methods
.relayRootBundle(relayerRefundRootArray, slowRelayRootArray)
.accounts(relayRootBundleAccounts)
Expand Down Expand Up @@ -220,7 +232,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
let relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
let relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [
Expand Down Expand Up @@ -351,7 +363,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
let relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
let relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [
Expand Down Expand Up @@ -492,7 +504,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
let relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
let relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [
Expand Down Expand Up @@ -554,7 +566,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
let relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
let relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [
Expand Down Expand Up @@ -616,7 +628,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
let relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
let relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [{ pubkey: relayerTA, isWritable: true, isSigner: false }];
Expand Down Expand Up @@ -706,7 +718,7 @@ describe("svm_spoke.bundle", () => {
const seeds = [Buffer.from("root_bundle"), state.toBuffer(), rootBundleIdBuffer];
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

const relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods
.relayRootBundle(relayerRefundRootArray, slowRelayRootArray)
.accounts(relayRootBundleAccounts)
Expand All @@ -722,6 +734,7 @@ describe("svm_spoke.bundle", () => {
state,
rootBundle,
signer: nonOwner.publicKey,
closer: nonOwner.publicKey,
program: program.programId,
};
await program.methods
Expand All @@ -735,7 +748,13 @@ describe("svm_spoke.bundle", () => {
}

// Execute the emergency delete
const emergencyDeleteRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const emergencyDeleteRootBundleAccounts = {
state,
rootBundle,
signer: owner,
closer: owner,
program: program.programId,
};
await program.methods.emergencyDeleteRootBundle(rootBundleId).accounts(emergencyDeleteRootBundleAccounts).rpc();

// Verify that the root bundle has been deleted
Expand Down Expand Up @@ -763,7 +782,13 @@ describe("svm_spoke.bundle", () => {
`Root bundle index should be ${initialRootBundleId + 1}`
);

const newRelayRootBundleAccounts = { state, rootBundle: newRootBundle, signer: owner, program: program.programId };
const newRelayRootBundleAccounts = {
state,
rootBundle: newRootBundle,
signer: owner,
payer: owner,
program: program.programId,
};
await program.methods
.relayRootBundle(newRelayerRefundRootArray, newSlowRelayRootArray)
.accounts(newRelayRootBundleAccounts)
Expand Down Expand Up @@ -869,7 +894,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
const relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

// Verify valid leaf
Expand Down Expand Up @@ -1021,7 +1046,7 @@ describe("svm_spoke.bundle", () => {
rootBundleIdBuffer.writeUInt32LE(rootBundleId);
const seeds = [Buffer.from("root_bundle"), state.toBuffer(), rootBundleIdBuffer];
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);
let relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
let relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();
const proofAsNumbers = proof.map((p) => Array.from(p));
const executeRelayerRefundLeafAccounts = {
Expand Down Expand Up @@ -1097,7 +1122,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
const relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [{ pubkey: relayerTA, isWritable: true, isSigner: false }];
Expand Down Expand Up @@ -1171,7 +1196,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
const relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

const remainingAccounts = [
Expand Down Expand Up @@ -1274,7 +1299,7 @@ describe("svm_spoke.bundle", () => {
const [rootBundle] = PublicKey.findProgramAddressSync(seeds, program.programId);

// Relay root bundle
const relayRootBundleAccounts = { state, rootBundle, signer: owner, program: program.programId };
const relayRootBundleAccounts = { state, rootBundle, signer: owner, payer: owner, program: program.programId };
await program.methods.relayRootBundle(Array.from(root), Array.from(root)).accounts(relayRootBundleAccounts).rpc();

// Pass refund addresses in remaining accounts.
Expand Down
Loading
Loading