Skip to content

[5/n][guardian-integration] unify Step 2 / Step 3 withdrawal timestamp#485

Closed
0xsiddharthks wants to merge 0 commit intosiddharth/guardian-integrationfrom
siddharth/guardian-commit-timestamp
Closed

[5/n][guardian-integration] unify Step 2 / Step 3 withdrawal timestamp#485
0xsiddharthks wants to merge 0 commit intosiddharth/guardian-integrationfrom
siddharth/guardian-commit-timestamp

Conversation

@0xsiddharthks
Copy link
Copy Markdown
Contributor

Stacked on #423 (siddharth/guardian-integration).

Summary

Step 2 (commitment pre-check) and Step 3 (guardian hard reserve) used different timestamps — Step 2 read the leader's checkpoint_timestamp_ms, while Step 3 read the on-chain WithdrawalTransaction.timestamp_ms set by Move's clock.timestamp_ms(). They could drift by seconds, and Step 2 could pass while Step 3 failed after the user's balance was already burned. This PR unifies them.

  • Rust: WithdrawalTxCommitment gains timestamp_ms: u64; the leader fills it with its checkpoint timestamp when building the commitment. Step 2's capacity_at now reads approval.timestamp_ms / 1000 — same source as Step 3.
  • Proto: SignWithdrawalTxConstructionRequest.timestamp_ms added.
  • Validator check: validate_withdrawal_tx_commitment rejects the commitment before BLS-signing if the leader's timestamp differs from this node's latest checkpoint by more than 5 minutes — catches both far-future (capacity theft) and far-past (stuck-forever DoS) values.
  • Move: WithdrawalCommitmentMessage carries timestamp_ms; commit_withdrawal_tx takes it as a parameter and includes it in the BLS-verified message. new_withdrawal_txn drops clock: &Clock in favour of timestamp_ms: u64, so the on-chain value is exactly what the committee signed.

Test

  • cargo nextest run -p hashi-types -p hashi-guardian -p hashi — 327/327.
  • sui move test in packages/hashi — 74/74.
  • make fmt && make clippy — clean.

@0xsiddharthks 0xsiddharthks requested a review from bmwill as a code owner April 23, 2026 14:27
Comment on lines +118 to +122
// Sui checkpoint timestamp (ms) observed by the leader when building
// the commitment. Stored verbatim on the on-chain
// `WithdrawalTransaction.timestamp_ms` so Step 2 pre-check and Step 3
// guardian hard reserve read the same value.
uint64 timestamp_ms = 5;
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.

We should avoid any timestamps that can be manipulated and are set other than by reading the onchain clock.

@0xsiddharthks 0xsiddharthks marked this pull request as draft April 23, 2026 15:11
@0xsiddharthks 0xsiddharthks changed the title [guardian] unify Step 2 / Step 3 withdrawal timestamp [5/n][guardian-integration] unify Step 2 / Step 3 withdrawal timestamp Apr 23, 2026
@0xsiddharthks 0xsiddharthks force-pushed the siddharth/guardian-commit-timestamp branch from 31d2f5e to e02a373 Compare April 23, 2026 18:26
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.

2 participants