Skip to content

chore: remove redundant sign status & bump checkpoint version #745

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.

Sign up for GitHub

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

Merged
ChaoticTempest merged 2 commits into from
Apr 10, 2026
Merged

chore: remove redundant sign status & bump checkpoint version #745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
330eaf8
Remove redundant status in BidirectionalTx
ChaoticTempest Apr 9, 2026
344d7d4
Bump storage version for checkpoint
ChaoticTempest Apr 9, 2026
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
220 changes: 173 additions & 47 deletions chain-signatures/node/src/backlog/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ mod tests {
use anchor_lang::prelude::Pubkey;
use mpc_primitives::{SignArgs, SignId};

fn create_test_tx(id: u8, status: SignStatus) -> BidirectionalTx {
fn create_test_tx(id: u8) -> BidirectionalTx {
BidirectionalTx {
id: BidirectionalTxId(B256::from([id; 32])),
sender: [0u8; 32],
Expand All @@ -850,7 +850,6 @@ mod tests {
request_id: [id; 32],
from_address: Address::ZERO,
nonce: 0,
status,
}
}

Expand Down Expand Up @@ -910,7 +909,12 @@ mod tests {
)
}

fn create_execution_entry(tx: BidirectionalTx, chain: Chain, dest: &str) -> BacklogEntry {
fn create_execution_entry(
tx: BidirectionalTx,
chain: Chain,
status: SignStatus,
dest: &str,
) -> BacklogEntry {
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

create_execution_entry constructs a BacklogEntry with an execution tx (Some(tx)), but it accepts an arbitrary status. That makes it easy for tests to create impossible states (e.g., AwaitingResponse with an execution tx), which can mask bugs. Consider making this helper always use SignStatus::PendingExecution (or renaming it to reflect that it can create inconsistent combinations / adding a debug assert that status != AwaitingResponse when execution is Some).

Suggested change
) -> BacklogEntry {
) -> BacklogEntry {
debug_assert!(
!matches!(status, SignStatus::AwaitingResponse),
"create_execution_entry cannot create an execution entry with AwaitingResponse status"
);

Copilot uses AI. Check for mistakes.
let sign_id = SignId::new(tx.request_id);
let request = IndexedSignRequest::new(
sign_id,
Expand All @@ -919,27 +923,27 @@ mod tests {
0,
SignKind::SignBidirectional(create_test_event(dest)),
);
BacklogEntry::with_status(request, tx.status, Some(tx))
BacklogEntry::with_status(request, status, Some(tx))
}

async fn insert_bidirectional_with_status(
backlog: &Backlog,
chain: Chain,
tx: BidirectionalTx,
status: SignStatus,
dest: &str,
) {
let sign_id = SignId::new(tx.request_id);
backlog
.insert(create_bidirectional_request(sign_id, chain, dest, 0))
.await;

match tx.status {
match status {
SignStatus::AwaitingResponse => {}
SignStatus::PendingExecution => {
backlog.advance(chain, sign_id, tx).await.unwrap();
}
SignStatus::Success | SignStatus::Failed => {
let status = tx.status;
backlog.advance(chain, sign_id, tx).await.unwrap();
backlog.set_status(chain, &sign_id, status).await;
}
Expand All @@ -950,19 +954,39 @@ mod tests {
async fn test_backlog_chain_isolation() {
let backlog = Backlog::new();

let tx_eth = create_test_tx(1, SignStatus::AwaitingResponse);
let tx_sol = create_test_tx(2, SignStatus::AwaitingResponse);
let tx_near = create_test_tx(3, SignStatus::AwaitingResponse);
let tx_eth = create_test_tx(1);
let tx_sol = create_test_tx(2);
let tx_near = create_test_tx(3);

let sign_id_eth = SignId::new(tx_eth.request_id);
let sign_id_sol = SignId::new(tx_sol.request_id);
let sign_id_near = SignId::new(tx_near.request_id);

// Insert into different chains
insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx_eth.clone(), "ethereum")
.await;
insert_bidirectional_with_status(&backlog, Chain::Solana, tx_sol.clone(), "solana").await;
insert_bidirectional_with_status(&backlog, Chain::NEAR, tx_near.clone(), "near").await;
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx_eth.clone(),
SignStatus::AwaitingResponse,
"ethereum",
)
.await;
insert_bidirectional_with_status(
&backlog,
Chain::Solana,
tx_sol.clone(),
SignStatus::AwaitingResponse,
"solana",
)
.await;
insert_bidirectional_with_status(
&backlog,
Chain::NEAR,
tx_near.clone(),
SignStatus::AwaitingResponse,
"near",
)
.await;

// Verify correct transactions in each chain
assert!(backlog.get(Chain::Ethereum, &sign_id_eth).await.is_some());
Expand All @@ -978,17 +1002,45 @@ mod tests {
let backlog = Backlog::new();

// Add transactions with different statuses to Ethereum
let tx1 = create_test_tx(1, SignStatus::AwaitingResponse);
let tx2 = create_test_tx(2, SignStatus::Success);
let tx3 = create_test_tx(3, SignStatus::PendingExecution);

insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx1, "ethereum").await;
insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx2, "ethereum").await;
insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx3, "ethereum").await;
let tx1 = create_test_tx(1);
let tx2 = create_test_tx(2);
let tx3 = create_test_tx(3);

insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx1,
SignStatus::AwaitingResponse,
"ethereum",
)
.await;
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx2,
SignStatus::Success,
"ethereum",
)
.await;
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx3,
SignStatus::PendingExecution,
"ethereum",
)
.await;

// Add transactions to Solana
let tx4 = create_test_tx(4, SignStatus::PendingExecution);
insert_bidirectional_with_status(&backlog, Chain::Solana, tx4, "solana").await;
let tx4 = create_test_tx(4);
insert_bidirectional_with_status(
&backlog,
Chain::Solana,
tx4,
SignStatus::PendingExecution,
"solana",
)
.await;

// Filter Ethereum by Pending
let eth_pending = backlog
Expand Down Expand Up @@ -1029,17 +1081,31 @@ mod tests {
for i in 0..5 {
let backlog = backlog.clone();
let handle = tokio::spawn(async move {
let tx = create_test_tx(i, SignStatus::AwaitingResponse);
insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx, "ethereum").await;
let tx = create_test_tx(i);
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx,
SignStatus::AwaitingResponse,
"ethereum",
)
.await;
});
handles.push(handle);
}

for i in 5..10 {
let backlog = backlog.clone();
let handle = tokio::spawn(async move {
let tx = create_test_tx(i, SignStatus::AwaitingResponse);
insert_bidirectional_with_status(&backlog, Chain::Solana, tx, "solana").await;
let tx = create_test_tx(i);
insert_bidirectional_with_status(
&backlog,
Chain::Solana,
tx,
SignStatus::AwaitingResponse,
"solana",
)
.await;
});
handles.push(handle);
}
Expand Down Expand Up @@ -1078,12 +1144,26 @@ mod tests {
let backlog = Backlog::new();

// Add some transactions
let tx1 = create_test_tx(1, SignStatus::PendingExecution);
let tx2 = create_test_tx(2, SignStatus::Success);
let tx1 = create_test_tx(1);
let tx2 = create_test_tx(2);
backlog.set_processed_block(Chain::Ethereum, 100).await;

insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx1.clone(), "ethereum").await;
insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx2.clone(), "ethereum").await;
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx1.clone(),
SignStatus::PendingExecution,
"ethereum",
)
.await;
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx2.clone(),
SignStatus::Success,
"ethereum",
)
.await;

let checkpoint = backlog.checkpoint(Chain::Ethereum).await;
assert_eq!(checkpoint.block_height, 100);
Expand All @@ -1093,27 +1173,47 @@ mod tests {

#[tokio::test]
async fn test_checkpoint_equality() {
let tx1 = create_test_tx(1, SignStatus::AwaitingResponse);
let tx2 = create_test_tx(2, SignStatus::AwaitingResponse);
let tx1 = create_test_tx(1);
let tx2 = create_test_tx(2);
let mut pending1 = PendingRequests::new();
pending1.insert(
SignId::new(tx1.request_id),
create_execution_entry(tx1.clone(), Chain::Ethereum, "ethereum"),
create_execution_entry(
tx1.clone(),
Chain::Ethereum,
SignStatus::AwaitingResponse,
"ethereum",
Comment on lines +1182 to +1185
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

These entries include an execution tx via create_execution_entry(..., Some(tx)), so using SignStatus::AwaitingResponse here represents a state that should not happen in production (execution is only set after advancing, which sets status to PendingExecution). Updating these to PendingExecution (and adjusting the helper accordingly) will make the checkpoint equality test reflect real backlog states.

Copilot uses AI. Check for mistakes.
),
);
pending1.insert(
SignId::new(tx2.request_id),
create_execution_entry(tx2.clone(), Chain::Ethereum, "ethereum"),
create_execution_entry(
tx2.clone(),
Chain::Ethereum,
SignStatus::AwaitingResponse,
"ethereum",
),
);
pending1.set_processed_block(100);

let mut pending2 = PendingRequests::new();
pending2.insert(
SignId::new(tx1.request_id),
create_execution_entry(tx1.clone(), Chain::Ethereum, "ethereum"),
create_execution_entry(
tx1.clone(),
Chain::Ethereum,
SignStatus::AwaitingResponse,
"ethereum",
),
);
pending2.insert(
SignId::new(tx2.request_id),
create_execution_entry(tx2.clone(), Chain::Ethereum, "ethereum"),
create_execution_entry(
tx2.clone(),
Chain::Ethereum,
SignStatus::AwaitingResponse,
"ethereum",
),
);
pending2.set_processed_block(100);

Expand All @@ -1130,12 +1230,17 @@ mod tests {

#[tokio::test]
async fn test_checkpoint_serialization() {
let tx1 = create_test_tx(1, SignStatus::AwaitingResponse);
let tx1 = create_test_tx(1);

let mut pending = PendingRequests::new();
pending.insert(
SignId::new(tx1.request_id),
create_execution_entry(tx1.clone(), Chain::Ethereum, "ethereum"),
create_execution_entry(
tx1.clone(),
Chain::Ethereum,
SignStatus::AwaitingResponse,
"ethereum",
Comment on lines +1238 to +1242
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

This test later calls take_execution_tx() and expects a "pending execution entry", but the entry is created with SignStatus::AwaitingResponse. To keep the serialized checkpoint representative of real data, set the status to PendingExecution (or use a helper that always sets that status when execution is present).

Copilot uses AI. Check for mistakes.
),
);
pending.set_processed_block(100);
let checkpoint = pending.checkpoint(Chain::Ethereum);
Expand Down Expand Up @@ -1165,9 +1270,16 @@ mod tests {
#[tokio::test]
async fn test_recover_restores_execution_watchers() {
let backlog = Backlog::new();
let tx = create_test_tx(6, SignStatus::PendingExecution);

insert_bidirectional_with_status(&backlog, Chain::Solana, tx.clone(), "ethereum").await;
let tx = create_test_tx(6);

insert_bidirectional_with_status(
&backlog,
Chain::Solana,
tx.clone(),
SignStatus::PendingExecution,
"ethereum",
)
.await;
backlog.set_processed_block(Chain::Solana, 10).await;

let checkpoint = backlog.checkpoint(Chain::Solana).await;
Expand Down Expand Up @@ -1247,7 +1359,7 @@ mod tests {
async fn test_watch_unwatch_and_set_status() {
use k256::Scalar;
let backlog = Backlog::new();
let tx = create_test_tx(7, SignStatus::PendingExecution);
let tx = create_test_tx(7);
let sign_id = SignId::new(tx.request_id);

// Insert a pending Sign request on the source chain
Expand Down Expand Up @@ -1296,8 +1408,15 @@ mod tests {
let backlog = Backlog::new();

// Add some transactions
let tx1 = create_test_tx(1, SignStatus::PendingExecution);
insert_bidirectional_with_status(&backlog, Chain::Ethereum, tx1.clone(), "ethereum").await;
let tx1 = create_test_tx(1);
insert_bidirectional_with_status(
&backlog,
Chain::Ethereum,
tx1.clone(),
SignStatus::PendingExecution,
"ethereum",
)
.await;

let interval = Chain::Ethereum.checkpoint_interval().unwrap();

Expand Down Expand Up @@ -1334,8 +1453,15 @@ mod tests {
let interval = Chain::Solana.checkpoint_interval().unwrap();

// Add transaction
let tx1 = create_test_tx(1, SignStatus::PendingExecution);
insert_bidirectional_with_status(&backlog, Chain::Solana, tx1.clone(), "solana").await;
let tx1 = create_test_tx(1);
insert_bidirectional_with_status(
&backlog,
Chain::Solana,
tx1.clone(),
SignStatus::PendingExecution,
"solana",
)
.await;

// Solana interval is 10 blocks
for i in 1..interval {
Expand All @@ -1354,7 +1480,7 @@ mod tests {
#[tokio::test]
async fn test_advance_rejects_plain_sign_entries() {
let backlog = Backlog::new();
let tx = create_test_tx(8, SignStatus::PendingExecution);
let tx = create_test_tx(8);
let sign_id = SignId::new(tx.request_id);

let args = SignArgs {
Expand Down
1 change: 0 additions & 1 deletion chain-signatures/node/src/sign_bidirectional.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ pub struct BidirectionalTx {
pub request_id: [u8; 32],
pub from_address: Address,
pub nonce: u64,
pub status: SignStatus,
}

impl BidirectionalTx {
Expand Down
2 changes: 1 addition & 1 deletion chain-signatures/node/src/storage/checkpoint_storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use tokio::sync::RwLock;
use std::collections::HashMap;
use std::sync::Arc;

const CHECKPOINT_VERSION: &str = "v2";
const CHECKPOINT_VERSION: &str = "v3";

#[derive(Clone, Debug)]
pub enum CheckpointStorage {
Expand Down
Loading
Loading