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): override chain id in executing slow fill #614

Merged
merged 2 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions programs/svm-spoke/src/instructions/slow_fill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,16 @@ pub fn execute_v3_slow_relay_leaf(
root_bundle_id: u32,
proof: Vec<[u8; 32]>,
) -> Result<()> {
let relay_data = slow_fill_leaf.relay_data.clone(); // Clone relay_data to avoid move
let relay_data = slow_fill_leaf.relay_data;

let slow_fill = V3SlowFill {
relay_data: relay_data.clone(), // Clone relay_data to avoid move
chain_id: ctx.accounts.state.chain_id, // This overrides caller provided chain_id, same as in EVM SpokePool.
updated_output_amount: slow_fill_leaf.updated_output_amount,
};

let root = ctx.accounts.root_bundle.slow_relay_root;
let leaf = slow_fill_leaf.to_keccak_hash();
let leaf = slow_fill.to_keccak_hash();
verify_merkle_proof(root, leaf, proof)?;

// Check if the fill status is unfilled
Expand Down
41 changes: 39 additions & 2 deletions test/svm/SvmSpoke.SlowFill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe("svm_spoke.slow_fill", () => {
};
}

const relaySlowFillRootBundle = async (slowRelayLeafRecipient = recipient) => {
const relaySlowFillRootBundle = async (slowRelayLeafRecipient = recipient, slowRelayLeafChainId = chainId) => {
//TODO: verify that the leaf structure created here is equivalent to the one created by the EVM logic. I think
// I've gotten the concatenation, endianness, etc correct but want to be sure.
const slowRelayLeafs: SlowFillLeaf[] = [];
Expand All @@ -89,7 +89,7 @@ describe("svm_spoke.slow_fill", () => {
exclusivityDeadline: new BN(Math.floor(Date.now() / 1000) - 60),
message: Buffer.from("Test message"),
},
chainId,
chainId: slowRelayLeafChainId,
updatedOutputAmount: new BN(relayAmount),
};
await updateRelayData(slowRelayLeaf.relayData);
Expand Down Expand Up @@ -499,4 +499,41 @@ describe("svm_spoke.slow_fill", () => {
assert.strictEqual(err.error.errorCode.code, "InvalidMint", "Expected error code InvalidMint");
}
});

it("Cannot execute V3 slow relay leaf targeted at another chain", async () => {
// Request V3 slow fill for another chain.
const anotherChainId = new BN(Math.floor(Math.random() * 1000000));
const { relayHash, leaf, rootBundleId, proofAsNumbers, rootBundle } = await relaySlowFillRootBundle(
undefined,
anotherChainId
);
await program.methods
.requestV3SlowFill(relayHash, leaf.relayData)
.accounts(requestAccounts)
.signers([relayer])
.rpc();

// Trying to execute V3 slow relay leaf for another chain should fail as the program overrides chain_id that should
// invalidate the proofs.
try {
await program.methods
.executeV3SlowRelayLeaf(relayHash, leaf, rootBundleId, proofAsNumbers)
.accounts({
state,
rootBundle,
signer: owner,
fillStatus: requestAccounts.fillStatus,
vault,
tokenProgram: TOKEN_PROGRAM_ID,
mint,
recipient,
recipientTokenAccount: recipientTA,
})
.rpc();
assert.fail("Execution should have failed for another chain");
} catch (err) {
assert.instanceOf(err, anchor.AnchorError);
assert.strictEqual(err.error.errorCode.code, "InvalidProof", "Expected error code InvalidProof");
}
});
});
Loading