Skip to content

feat: resubmit expired transactions with new blockhash#56

Merged
lpahlavi merged 14 commits intomainfrom
lpahlavi/resubmit-transactions-v2
Mar 24, 2026
Merged

feat: resubmit expired transactions with new blockhash#56
lpahlavi merged 14 commits intomainfrom
lpahlavi/resubmit-transactions-v2

Conversation

@lpahlavi
Copy link
Copy Markdown
Contributor

@lpahlavi lpahlavi commented Mar 18, 2026

(DEFI-2670) Add timer task that resubmits all expired submitted transactions with a new blockhash.

Deterministic integration tests similar to the ones for deposit consolidation task added in #60 will be added in a follow-up PR.

@lpahlavi lpahlavi force-pushed the lpahlavi/resubmit-transactions-v2 branch from 5f7b63e to 91a1d7d Compare March 19, 2026 08:46
@lpahlavi lpahlavi force-pushed the lpahlavi/resubmitted-transaction-event branch 2 times, most recently from 6a2b658 to de1863e Compare March 19, 2026 14:57
@lpahlavi lpahlavi force-pushed the lpahlavi/resubmit-transactions-v2 branch from 12ece65 to d4b5182 Compare March 20, 2026 08:16
@lpahlavi lpahlavi changed the title feat: resubmit transactions with new blockhash feat: resubmit expired transactions with new blockhash Mar 20, 2026
Base automatically changed from lpahlavi/resubmitted-transaction-event to main March 20, 2026 13:37
lpahlavi and others added 8 commits March 20, 2026 15:50
Add tests for the consolidate_deposits function covering:
- Early return when no funds to consolidate
- Early return when task already active (guard)
- Early return when fetching blockhash fails
- Single consolidation request with event assertions
- Multiple consolidation batches (11 accounts → 2 batches)

Also adds canister_self() to CanisterRuntime trait to enable
mocking in tests, and extracts MAX_TRANSFERS_PER_CONSOLIDATION
constant for test visibility.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Record `ConsolidatedDeposits` and `SubmittedTransaction` events before
calling `submit_transaction` to ensure they persist for resubmission
even if the RPC call fails.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add timer task that resubmits all pending transactions every 60 seconds
with a fresh blockhash. Uses ResubmittedTransaction event to track the
signature change and updated slot.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tests for the resubmit_transactions function covering:
- Early return when no transactions to resubmit
- Early return when task already active (guard)
- Early return when fetching current slot fails
- No resubmission when transaction not expired
- Single expired transaction resubmission with event assertions
- Event recorded even when submission fails

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@lpahlavi lpahlavi force-pushed the lpahlavi/resubmit-transactions-v2 branch from e2f05f1 to 39855fc Compare March 20, 2026 15:31
@lpahlavi lpahlavi changed the base branch from main to lpahlavi/consolidation-timer-tests March 20, 2026 15:33
@lpahlavi lpahlavi marked this pull request as ready for review March 20, 2026 16:03
@lpahlavi lpahlavi requested a review from a team as a code owner March 20, 2026 16:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an automated resubmission loop for Solana transactions that have expired due to blockhash age, wiring it into the canister’s timer infrastructure.

Changes:

  • Introduce a resubmit module with logic to re-sign expired submitted transactions using a fresh recent blockhash and submit them again.
  • Add a new timer (RESUBMIT_TRANSACTIONS_DELAY) to periodically run the resubmission task guarded by TimerGuard.
  • Extend state/task plumbing for the new timer task and add unit tests around resubmission behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
minter/src/resubmit/mod.rs New resubmission timer task implementation (fetch slot/blockhash, batch, re-sign, submit, audit event).
minter/src/resubmit/tests.rs New unit tests for early-return paths and single-transaction resubmission behavior.
minter/src/state/mod.rs Expose submitted_transactions() accessor and add TaskType::ResubmitTransactions.
minter/src/main.rs Register new timer interval to invoke resubmit_transactions.
minter/src/lib.rs Export pub mod resubmit.
minter/src/test_fixtures/mod.rs Align MINTER_ACCOUNT owner with the test runtime canister id.
minter/src/sol_transfer/mod.rs Minor test module placement adjustment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread minter/src/monitor/mod.rs
Comment thread minter/src/monitor/mod.rs
Comment thread minter/src/resubmit/mod.rs Outdated
Comment thread minter/src/resubmit/mod.rs Outdated
Comment thread minter/src/monitor/mod.rs
let current_slot = match get_slot(&runtime).await {
Ok(slot) => slot,
Err(e) => {
log!(Priority::Info, "Failed to get current slot: {e}");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to introduce the Error priority for such cases?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. IMO this is expected to happen once in a while (e.g. due to provider or connectivity issues) so it's not really an error per se. I would reserve the error priority for something that would need to be looked at. WDYT?

Comment thread minter/src/monitor/mod.rs
.collect::<Vec<_>>()
});

while !expired_transactions.is_empty() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similarly to your comment in PR #48, don't we want to limit the number of resubmitted transactions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, I think we can assume that we don't run out of cycles, as long as we don't submit too many transactions in parallel.

Comment thread minter/src/monitor/mod.rs
return;
}
};
let new_slot = match get_slot(&runtime).await {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once estimate_recent_blockhash is extended to return also the slot, we will refactor this to single call for (new_slot, new_blockhash), is that correct? In that case, maybe add a TODO?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Done.

Rename `resubmit` to `monitor`, `resubmit_transactions` to
`monitor_submitted_transactions`, and `ResubmitTransactions` to
`MonitorSubmittedTransactions`. No logic changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Base automatically changed from lpahlavi/consolidation-timer-tests to main March 24, 2026 14:06
lpahlavi and others added 2 commits March 24, 2026 15:11
…nsactions-v2

# Conflicts:
#	minter/src/consolidate/mod.rs
#	minter/src/consolidate/tests.rs
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 24, 2026 14:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread minter/src/monitor/mod.rs
Comment on lines +194 to +213
fn minter_address() -> solana_address::Address {
use crate::state::SchnorrPublicKey;
let master_key = SchnorrPublicKey {
public_key: PublicKey::pocketic_key(PocketIcMasterPublicKeyId::Key1),
chain_code: [1; 32],
};
derive_public_key(&master_key, vec![])
.serialize_raw()
.into()
}

fn add_submitted_transaction(signature: Signature, slot: Slot) {
let message = Message::new_with_blockhash(&[], Some(&minter_address()), &Hash::default());
mutate_state(|state| {
process_event(
state,
EventType::SubmittedTransaction {
signature,
transaction: message,
signers: vec![MINTER_ACCOUNT],
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The test helper builds a Message whose fee payer is minter_address() derived from an empty derivation path, but the signers recorded in the SubmittedTransaction event are vec![MINTER_ACCOUNT] (which uses derivation_path(&MINTER_ACCOUNT) in production code). This mismatch means the test isn’t constructing a transaction message that corresponds to the signer derivation paths used by resubmit_transaction_with_new_blockhash, so it can’t catch bugs where re-signing produces a signature that doesn’t match the message’s fee payer/signers. Consider deriving the fee payer address from the same master key and derivation_path(&MINTER_ACCOUNT) (or pulling the master key from state) so the message and signers are consistent.

Copilot uses AI. Check for mistakes.
@lpahlavi lpahlavi merged commit 267f690 into main Mar 24, 2026
14 checks passed
@lpahlavi lpahlavi deleted the lpahlavi/resubmit-transactions-v2 branch March 24, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants