[Certora] OfferTree Soundness - #816
Conversation
Co-authored-by: Quentin Garchery <garchery.quentin@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Quentin Garchery <garchery.quentin@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
`ERC20NoReturn.approve` was declared `returns (bool)` and returned `true`, so
the TOKEN_TYPE=4 ("no-return") matrix leg only exercised no-return
`transfer`/`transferFrom` and never a void `approve`.
Voiding the mock's `approve` makes every high-level `approve` call declared
`returns (bool)` revert on solc's returndatasize check, so test approvals on
matrix tokens now go through `ERC20Lib.safeApprove`, which accepts empty-or-
`true` returndata. All five matrix mocks return `true` unconditionally (USDT
reverts, never returns `false`), so this is behaviour-preserving; the `require`
wrappers dropped in BlueBuyCallbackIntegrationTest were never falsifiable and
`safeApprove` retains the equivalent check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TFq2E5QUuD8fSDyvpQ6BkQ
Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HC6jESoBGvyJkzKEwm3n9t
Drop the decreasing branch of the incentive ramp in `roll` and require `incentiveAtEnd >= incentiveAtStart` in `setConfig`, so the incentive can only increase over the rolling window.
Signed-off-by: PA <50184410+peyha@users.noreply.github.com>
Pins foundry-rs/foundry-toolchain to the v1.9.1 commit 908c540300062bd5a7e473851cdb4282204cee09.\n\nWhy now: foundryup became a Rust binary in foundry-rs/foundry#15498, while the v1.7 bootstrap program still invokes it as a bash script. The bootstrap fetches the latest foundryup, so the unpinned action can now break CI.\n\nVerification: workflow and local composite-action files containing foundry-rs/foundry-toolchain were updated; tests not run (workflow ref update only).\n\nRequested by: <@U03C031MDG8> | Quentin Garchery
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 008b787b61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| require(leaves.length > 0 && (leaves.length & (leaves.length - 1)) == 0, "invalid leaves length"); | ||
|
|
||
| bytes32[] memory level = new bytes32[](leaves.length); | ||
| for (uint256 i = 0; i < leaves.length; i++) { |
There was a problem hiding this comment.
Cover every offer processed by generateRoot
When leaves contains four or more entries, this loop exceeds OfferTreeWellFormed.conf's optimistic loop_iter: 3 bound, so those executions are pruned rather than checked during preservation of the wellFormed invariant. Consequently the proof covers generateRoot only for the smallest accepted arrays, even though the public helper accepts arbitrary non-empty power-of-two lengths; summarize this loop or constrain and document the supported input size.
AGENTS.md reference: AGENTS.md:L13-L14
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0e2d9281b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "optimistic_loop": true, | ||
| "loop_iter": 3, | ||
| "optimistic_hashing": true, | ||
| "hashing_length_bound": 1024, |
There was a problem hiding this comment.
Cover callback data beyond the hashing bound
For an otherwise valid offer whose callbackData exceeds 1024 bytes, HashLib.hashOffer hashes that dynamic byte array but this optimistic hashing bound prunes the full execution, so membershipSoundness does not cover all offers accepted by take. The same bound appears in OfferTreeWellFormed.conf; either constrain and document callback-data length in both properties or model its hash without this bound.
Useful? React with 👍 / 👎.
Objective is to show that a successful
takecan only settle an offer that was genuinely committed in the signed tree.We reason about
OfferTree, a model of the tree built only through thenewLeafandnewInternalNodeprimitives. Leaves are keyed byHashLib.hashOffer(offer)and store a fixed-size pre-image of the offer, soisWellFormedre-hashes a leaf with a single bounded keccak instead of looping over the offer's dynamic members, which keeps the proofs bounded regardless of offer size.OfferTreeWellFormed.specchecks that the primitives only ever build well-formed trees: every node is empty, a leaf carrying a genuinehashOffer, or an internal node correctly hashing its two children.OfferTreeMembership.specchecks the main soundness result: for any well-formed tree, if a Merkle proof verifies an offer's hash against the root viaisLeaf, then the offer is registered as a leaf. Equivalently, no valid proof can be forged for an offer that is not in the tree.The verification setup and technique is inspired from the Merkle Tree Membership soundness spec in Universal Rewards Distributor
corresponding thread