Skip to content

Commit

Permalink
Update used_nonces PDA seeds (#25)
Browse files Browse the repository at this point in the history
There is a PDA seed collision on the `used_nonces` PDA with specific
(domain, nonce) combinations due to the fact that both seeds are
variable length. The fix is to apply a delimiter between the seeds. This
fix is only applied to domains >= 11 because domain 11 is the first time
this collision can occur and all existing PDAs must stay the same with
this change.

Adding a test testing the collision that would've failed before the fix
is applied and passes with the fix.

Testnet program has been upgraded with these changes, mainnet will be
upgraded in January.
  • Loading branch information
chasemcdermott authored Dec 13, 2024
1 parent d9a7daa commit 8b115e4
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub fn get_nonce_pda(
&[
b"used_nonces",
params.source_domain.to_string().as_bytes(),
UsedNonces::used_nonces_seed_delimiter(params.source_domain),
UsedNonces::first_nonce(params.nonce)?
.to_string()
.as_bytes(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub struct ReceiveMessageContext<'info> {
seeds = [
b"used_nonces",
Message::new(message_transmitter.version, &params.message)?.source_domain()?.to_string().as_bytes(),
UsedNonces::used_nonces_seed_delimiter(Message::new(message_transmitter.version, &params.message)?.source_domain()?),
UsedNonces::first_nonce(Message::new(message_transmitter.version, &params.message)?.nonce()?)?.to_string().as_bytes()
],
bump
Expand Down
11 changes: 11 additions & 0 deletions programs/message-transmitter/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,15 @@ impl UsedNonces {

Ok((entry, bit))
}

/// Adds a delimiter for the used_nonces PDA seeds for domains >= 11
/// Only add to domains >= 11 to prevent existing (pre-upgrade on mainnet)
/// PDAs from changing.
pub fn used_nonces_seed_delimiter(source_domain: u32) -> &'static [u8] {
if source_domain < 11 {
b""
} else {
b"-"
}
}
}
12 changes: 12 additions & 0 deletions tests/message-transmitter/test_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,16 @@ export class TestClient {
.signers([this.attesterManager])
.rpc();
};

getNoncePda = async (nonce: number, remoteDomain: number): Promise<PublicKey> => {
return this.program.methods
.getNoncePda({
nonce: new anchor.BN(nonce),
sourceDomain: remoteDomain
})
.accounts({
messageTransmitter: this.messageTransmitter.publicKey
})
.view();
};
}
15 changes: 15 additions & 0 deletions tests/message-transmitter/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { PublicKey } from "@solana/web3.js";
import { expect, assert } from "chai";
import * as ethutil from "ethereumjs-util";
import BN from "bn.js";
import { generateUsedNoncesCollisions } from "../utils";

describe("message_transmitter", () => {
let tc = new TestClient();
Expand Down Expand Up @@ -253,4 +254,18 @@ describe("message_transmitter", () => {
await tc.setSignatureThreshold(2);
await tc.updateAttesterManager(tc.provider.wallet.publicKey);
});

it("used_nonces PDA seeds do not cause collision", async () => {
// Generate a bunch of used nonces collisions
const collisions = generateUsedNoncesCollisions(500);
expect(collisions.length).to.equal(500);

// Make sure none of them collide
await Promise.all(collisions.map(async ({nonce1, nonce2, domain1, domain2}) => {
const pda1 = await tc.getNoncePda(nonce1, domain1);
const pda2 = await tc.getNoncePda(nonce2, domain2);

expect(pda1.toString()).not.to.equal(pda2.toString());
}));
})
});
25 changes: 25 additions & 0 deletions tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,28 @@ export function getEvent(events, program: PublicKey, eventName: string) {
}
throw new Error("Event " + eventName + " not found");
}

export function generateUsedNoncesCollisions(numCollisions = 500) {
const set = new Set();
const map = {};
const collisions = [];

// source domains 0-50
for (let i = 0; i < 50; i++) {
// nonces 1-200M. Increment by 6400 since only the first nonce is in the seed
for (let j = 1; j < 20000000; j+=6400) {
if (set.has(`${i}${j}`)) {
// Collision
collisions.push({nonce1: map[`${i}${j}`].j, domain1: map[`${i}${j}`].i, nonce2: j, domain2: i});
} else {
// No collision, add to the set and map
set.add(`${i}${j}`);
map[`${i}${j}`] = {i, j}
}
if (collisions.length >= numCollisions) {
return collisions;
}
}
}
return collisions;
}

0 comments on commit 8b115e4

Please sign in to comment.