Skip to content

docs: clarify timestamp precedence governs expiration, not amounts#62

Open
re1ro wants to merge 1 commit into
mainfrom
docs/clarify-timestamp-precedence
Open

docs: clarify timestamp precedence governs expiration, not amounts#62
re1ro wants to merge 1 commit into
mainfrom
docs/clarify-timestamp-precedence

Conversation

@re1ro

@re1ro re1ro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

The README and docs stated that signed timestamps "order operations across chains" and that "most recent timestamp takes precedence in expiration updates." Taken together this is misleading: it suggests cross-chain timestamp ordering also serializes/orders allowance amount changes. It does not.

Verified against src/Permit3.sol:

  • _processIncreaseOrUpdate (lines ~464-492): amountDelta is applied unconditionally and before the timestamp comparison. The timestamp check (timestamp > allowed.timestamp) only updates expiration and the stored timestamp.
  • _decreaseAllowance: subtracts amountDelta unconditionally, with no timestamp logic at all.
  • Replay protection (running the same signed operation twice) comes from the per-salt, non-sequential nonce, not from timestamp ordering.

So the precise statement is: timestamps order expiration updates, not amount changes.

Files changed

  • README.md — rewrote the "Timestamp Management" bullets.
  • docs/concepts/allowance-system.md — reframed the Timestamp Management "Purpose"/"Rules" and the "Operation Priority" list; added a note that amounts apply unconditionally and replay is nonce-based.
  • docs/concepts/architecture.md — "Timestamp Ordering" -> "Timestamp-Ordered Expirations".
  • docs/concepts/cross-chain-operations.md — clarified the security-considerations bullet.
  • docs/api/data-structures.md — corrected the timestamp field description.
  • docs/api/events.md — corrected three operation-ordering references.
  • docs/api/api-reference.md — corrected the "Timestamp Ordering" security bullet.

Notes

This is a docs-only change; no .sol source was modified.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 23, 2026 17:48
@eco-ai-app

eco-ai-app Bot commented Jun 23, 2026

Copy link
Copy Markdown

Overview

This PR primarily updates documentation to clarify timestamp semantics, emphasizing that timestamp precedence governs the expiration field only and does not order amount changes. It also includes two minor Solidity fixes: correcting the EIP-712 encoding of the dynamic salts array in NonceManager.sol and removing an unused _cachedChainId immutable in EIP712.sol. The review team analyzed the changes for security, performance, and QA concerns, and found no issues.

QA & Test Coverage

No issues found in this domain.

Web Security

Skipped: not applicable to this PR — PR is documentation-only with no web-facing code changes

Blockchain Security

No issues found in this domain.

Performance

No issues found in this domain.

Stats

Metric Value
Reviewers 4
Successful 3
Skipped 1
Failed 0
Total findings 0
Cross-domain 0
Tokens (in / out) 115080 / 2465

Automated review by Eco's specialized review team. Architecture, business logic, and correctness remain with the human reviewer.

The README and docs implied that signed timestamps "order operations
across chains," which is misleading. The on-chain behavior in
Permit3._processIncreaseOrUpdate is:

- amountDelta is applied unconditionally before the timestamp check;
  each Increase adds and each Decrease subtracts regardless of order.
- The timestamp precedence rule (most-recent-wins) governs only the
  expiration field and the stored timestamp.
- Replay protection comes from per-salt non-sequential nonces, not
  from timestamp ordering.

Reword the README "Timestamp Management" section and the related
concept/API docs to state this precisely so integrators do not assume
cross-chain timestamp ordering serializes balance changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@re1ro re1ro force-pushed the docs/clarify-timestamp-precedence branch from c338da3 to 30c97d0 Compare June 23, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR primarily updates the documentation to clarify that signed timestamp precedence governs expiration updates only, and does not serialize/order allowance amount changes; amount deltas apply unconditionally per operation and replay protection is nonce-based. However, the PR also includes Solidity changes that affect EIP-712 hashing for nonce invalidation, which is a behavioral/API change beyond “docs-only”.

Changes:

  • Reframed docs/README explanations of timestamp precedence to apply only to expiration, and clarified unconditional amount deltas + nonce-based replay protection.
  • Updated cross-chain/security documentation references to avoid implying timestamp-ordered amount updates.
  • Modified hashNoncesToInvalidate hashing logic (and adjusted EIP-712 caching commentary), impacting off-chain signing/proof derivations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/NonceManager.sol Changes EIP-712 struct hashing for NoncesToInvalidate (affects signature/proof compatibility).
src/lib/EIP712.sol Updates domain-separator caching commentary and removes unused cached chain-id state.
README.md Clarifies timestamp precedence applies to expiration only; amounts are unconditional; replay is nonce-based.
docs/concepts/allowance-system.md Reframes “Timestamp Management” to explicitly scope precedence to expirations and explain amount/replay behavior.
docs/concepts/architecture.md Renames/clarifies “Timestamp Ordering” as “Timestamp-Ordered Expirations”.
docs/concepts/cross-chain-operations.md Clarifies timestamp precedence in security best practices (but surrounding domain-separation text remains inconsistent).
docs/api/data-structures.md Corrects Allowance.timestamp field description to reflect expiration-only precedence.
docs/api/events.md Updates event interpretation guidance to “expiration precedence” rather than general operation ordering.
docs/api/api-reference.md Updates security bullet to reflect expiration-only precedence + nonce-based replay protection.

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

Comment on lines 362 to 366
### 4. **Additional Security Best Practices**
- **Deadline Validation**: Set reasonable deadlines to limit the window of vulnerability
- **Timestamp Ordering**: Be aware of timestamp-based operation ordering across chains
- **Timestamp Ordering**: Be aware that timestamp precedence orders `expiration` updates across chains only — amount changes apply unconditionally and replay is prevented by per-salt nonces
- **Proof Verification**: Ensure proofs correctly link chain-specific permits to the root hash
- **Witness validation**: When using witness data, ensure proper validation on each chain
@MSevey MSevey requested a review from fish-sammy June 24, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants