Skip to content

fix(contracts): move consumedIntents assignment before external call (CEI)#70

Merged
iap merged 1 commit into
devfrom
fix/cei-consumed-intents
May 10, 2026
Merged

fix(contracts): move consumedIntents assignment before external call (CEI)#70
iap merged 1 commit into
devfrom
fix/cei-consumed-intents

Conversation

@iap

@iap iap commented May 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a CEI (Checks-Effects-Interactions) pattern violation in MARKSettlementModule._consumeAndValidate.

Root cause

consumedIntents[intentId] = true was set after the external call to verifier_.verifySettlement(). The current verifier is view so reentrancy is not possible today, but this would become a real reentrancy vector if a future non-view verifier is plugged in.

Fix

Move consumedIntents[intentId] = true to before the external call. No behaviour change for the current verifier.

Verification

All tests pass.

Scope

  • contracts/src/settlement/MARKSettlementModule.sol
  • contracts/THREAT_MODEL.md

Risk

Low — no behaviour change for current verifier. Improves correctness for future verifier integrations.

Summary by CodeRabbit

  • Bug Fixes

    • Adjusted intent consumption validation order in settlement operations to enhance protection against potential intent reuse.
  • Documentation

    • Updated protocol invariants documentation regarding intent consumption validation and verification ordering to align with implementation.

Review Change Stack

…(CEI)

Follows Checks-Effects-Interactions pattern by marking the intent as
consumed before calling verifier_.verifySettlement(). The current
verifier is view so reentrancy is not possible today, but this makes
the code correct by construction for any future non-view verifier.

Also updates THREAT_MODEL.md invariant 2 to reflect the fix.
@iap iap requested a review from a team as a code owner May 10, 2026 15:47
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 63b1f33b-f41a-43a1-b905-cb8aa1ba8da9

📥 Commits

Reviewing files that changed from the base of the PR and between 3b5720b and c39890b.

📒 Files selected for processing (2)
  • contracts/THREAT_MODEL.md
  • contracts/src/settlement/MARKSettlementModule.sol

Walkthrough

This PR reorders the intent consumption check in _consumeAndValidate to set consumedIntents[intentId] = true before the optional external verifier call, implementing the CEI (Checks-Effects-Interactions) pattern for replay and reentrancy protection. The threat model documentation is updated to reflect this ordering.

Changes

Intent Consumption Replay Protection

Layer / File(s) Summary
Core Implementation
contracts/src/settlement/MARKSettlementModule.sol
In _consumeAndValidate, consumedIntents[intentId] = true is moved to execute before the optional external verifier_.verifySettlement() call, ensuring intent consumption is recorded in state (Effects phase) before external interactions occur.
Threat Model Documentation
contracts/THREAT_MODEL.md
Invariant #2 updated to document the revised ordering: consumedIntents[intentId] is set to true before the external verifier call, with explicit reference to CEI pattern for replay/reentrancy protection in case future verifier implementations become non-view.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • trade/mark#68: Adds the initial THREAT_MODEL.md; this PR updates that document alongside the corresponding code change.
  • trade/mark#69: Both PRs modify the same invariant and _consumeAndValidate consumption/validation ordering but make opposing changes to the timing of consumedIntents assignment.
  • trade/mark#26: Adds tests for settlement verification and consumption behavior; related to the verification/consumption changes in this PR.

Poem

🐰 Intent once consumed, cannot be twice,
CEI patterns keep the protocol nice,
Mark state before calling out,
Replay's what we're guarding about!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: moving consumedIntents assignment before an external call to follow CEI pattern.
Description check ✅ Passed The description covers summary, root cause, fix, verification, scope, and risk assessment, addressing most template sections comprehensively.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cei-consumed-intents

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

@iap iap merged commit 8627f4b into dev May 10, 2026
19 checks passed
@iap iap deleted the fix/cei-consumed-intents branch May 10, 2026 15:51
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.

1 participant