Skip to content

feat: Implement payroll execution idempotency and replay protection - #232

Open
OG-wura wants to merge 7 commits into
zkpayroll:mainfrom
OG-wura:Implement_payroll_execution
Open

feat: Implement payroll execution idempotency and replay protection#232
OG-wura wants to merge 7 commits into
zkpayroll:mainfrom
OG-wura:Implement_payroll_execution

Conversation

@OG-wura

@OG-wura OG-wura commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Close #165

Implement Payroll Execution Idempotency & Replay Protection

Summary

Adds contract-level protections that prevent duplicate payroll execution for the same company, payroll period, commitment batch, and treasury authorization context. Payroll execution is now safe under retries, delayed confirmations, client crashes, and malicious replay attempts.

Changes

contracts/payment_executor/src/lib.rs

  • DataKey::BatchExecution(BytesN<32>) — tracks whether a specific batch fingerprint has been executed
  • DataKey::BatchExecutionRecords(BytesN<32>) — caches Vec<PaymentRecord> for idempotent returns
  • compute_batch_fingerprint() — SHA-256 hash of (company_id, period, employees, amounts, nullifiers) using soroban_sdk::Bytes + env.crypto().sha256()
  • execute_batch_payroll() — computes fingerprint upfront; if batch already executed, returns cached records without re-execution (safe idempotent retry); emits BatchExecuted event on first execution
  • Tests: test_execute_batch_idempotent_retry, test_different_batches_have_distinct_identities

contracts/payroll/src/lib.rs

  • DataKey::NonceFingerprint(BytesN<32>) — stores SHA-256 batch fingerprint alongside consumed nonce
  • compute_batch_fingerprint() — SHA-256 hash of (total_spend, employees, amounts, proof prefixes)
  • batch_process_payroll() — idempotent nonce check:
    • Same nonce + same fingerprint → returns existing run_id (safe retry)
    • Same nonce + different fingerprint → panics with "Conflicting replay detected: nonce already used with a different batch payload" (malicious replay)
  • prepare_payroll_run() — same idempotent/replay logic applied to the prepare path
  • Tests (6 new):
    • test_idempotent_retry_same_batch_returns_same_run_id — retry returns same run_id
    • test_conflicting_replay_with_modified_payload_is_rejected — different payload rejected
    • test_prepare_payroll_run_idempotent_retry — prepare path idempotent
    • test_prepare_payroll_run_rejects_conflicting_replay — prepare path replay rejected
    • test_distinct_nonces_with_same_payload_are_distinct_runs — different nonces = different runs
    • test_cross_company_same_nonce_does_not_collide — same nonce on different contract instances is independent

contracts/payroll/src/libmain.rs

  • Added NonceFingerprint(BytesN<32>) DataKey variant for consistency

docs/security/replay-protection.md (new)

Comprehensive documentation covering:

  • All protection layers (per-proof nullifier, per-employee period guard, batch nonce, payload fingerprint, batch execution identity)
  • Canonical execution identity definition
  • Safe retry vs malicious replay flow diagrams
  • Integrator guidance (nonce selection, error handling, retry strategy with exponential backoff)
  • Cross-company collision safety explanation
  • Storage key design reference
  • Testing checklist

docs/events.md

  • Added BatchExecuted event to schema reference

Execution Identity Design

The canonical execution identity is a SHA-256 fingerprint computed from:

SHA-256(
    expected_total_spend (16 bytes BE)
    + for each employee i:
        employee_address (32 bytes XDR)
        + amount (16 bytes BE)
        + proof_hash (first 32 bytes)
)

Two batches with identical parameters always produce the same fingerprint; any change produces a different one.

Acceptance Criteria

Criteria Status
Re-submitting exact same completed payroll does not duplicate state
Replaying with modified payload data is rejected
Execution identity cannot collide across companies
Tests cover success, retry, duplicate rejection, malicious replay
Contract errors are explicit enough for SDK mapping
Documentation explains client-side retry behavior

Testing

cargo test -p payment_executor   # 2 new idempotency tests
cargo test -p payroll            # 6 new idempotency/replay tests

@drips-wave

drips-wave Bot commented Jul 28, 2026

Copy link
Copy Markdown

@OG-wura Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

OG-wura added 6 commits July 29, 2026 15:28
Resolved merge conflicts in contracts/payroll/src/lib.rs:
- Kept origin/main's metadata hash verification tests (zkpayroll#177)
- Added HEAD's idempotent retry & replay protection tests after them
- Both test suites are preserved in the resolved file
- Resolved merge conflicts in contracts/payroll/src/lib.rs
- Fixed no_std format! usage in payment_executor and payroll_registry
- Added missing ToXdr imports for to_xdr() calls
- Fixed sha256() return type (Hash<32> → BytesN<32> via .into())
- Fixed fingerprint clone-after-move in payment_executor
- Added registry field to ContractAddresses in libmain.rs
- Removed .unwrap() from execute_batch_payroll test calls (auto-unwrapped)
- Added Events testutils import for env.events().all()
Resolve conflicts in payment_executor, payroll, and payroll_registry:
- payment_executor: keep treasury asset mapping validation (issue zkpayroll#217)
  alongside batch execution idempotency; preserve both test suites.
- payroll: combine idempotency/replay protection tests with treasury
  deposit, archived run, and company state gate tests; adapt nonce tests
  to the new idempotent retry semantics.
- payroll_registry: adopt add_employee_by_wallet refactor; drop
  duplicate add_employee_record body.

Follow-up fixes so the merged tree builds and tests pass:
- Update payment_executor and integration_tests event-count assertions
  for the TreasuryAssetAllowedUpdated and run_state events.
- Fix idempotency-related payroll tests for the new safe-retry behavior.
- Remove invalid AuditModuleClient::initialize call in migration helpers.
- Fix clippy clone-on-copy in payroll; dedupe BatchExecuted docs row.
…ecks

- cargo fmt --all: reformat workspace drift that failed the Rustfmt and
  Format & Clippy checks (rustfmt now requires the changes CI expects).
- clippy: remove unused imports/variables in migration test crates, allow
  documented-but-unused fixture constants, drop redundant Events import and
  unused treasury_owner bindings in payroll tests, fix assert_eq! bool
  literals and module_inception in integration_tests fixtures.
- migration_tests: set a nonzero ledger timestamp in setup so fixtures write
  valid executed_at/requested_at timestamps; fix mg_03 company ID expectation
  to match 0-based register_company; register both companies in mg_14/mg_15 so
  the post-upgrade company lookups succeed; keep registry commitment in sync
  after the simulated rotation in mg_02; drop full-state migration helper
  calls from the minimal-state mg_14/mg_15 setups.

All workspace tests pass: payroll 110, payroll_registry 41, audit_module 38,
payment_executor 37, migration_tests 21, integration_tests 27, etc.
@edehvictor

Copy link
Copy Markdown
Contributor

@OG-wura, kindly resolve conflicts.

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.

Implement payroll execution idempotency and replay protection

2 participants