You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: high. Not approving: this introduces the core Escrow collateral contract (security-sensitive scope) and exceeds the medium-risk approval threshold. Cursor Bugbot and Cursor Security Agent checks were not present; human review is needed for unresolved feedback on Escrow.sol.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Implement the full BYOSEscrow contract with owner/operator role
separation, cooldown-based all-or-nothing withdrawals, freeze/unfreeze
for Track B investigations, debit authority for Track A/B penalties,
and permissionless debit sweeping to owner.
The block-timestamp lint warns about validator manipulation of
block.timestamp, but this is irrelevant for day-scale cooldowns.
Foundry 1.7.1 runs the linter during build and deny=warnings makes
this a hard error in CI.
…t tests
Extend test_full_withdrawal_lifecycle with a second deposit/withdraw
cycle to verify no leftover deposits or totalDebited in contract state.
Add test_debit_after_withdrawal_reverts to confirm operator cannot
debit a sub-solver after their balance has been fully withdrawn.
Replace direct ownership transfer with a two-step process: owner calls
transferOwnership to set a pendingOwner, then pendingOwner calls
acceptOwnership to finalize. Prevents irrecoverable loss from typos.
A second transferOwnership call overrides any pending transfer.
Consolidates the repeated deposits[s] - totalDebited[s] subtraction
into a single internal function with an explicit underflow guard.
Reverts with InsufficientBalance instead of a raw panic if the
invariant is ever violated.
Collapses two per-sub-solver mappings into one. Deposits add to the
balance, debits subtract from it, withdrawal zeroes it. Eliminates
indefinite accumulation of stale accounting state, removes the
_balance() helper (no subtraction needed), and saves one SLOAD in
debit() and one SSTORE in executeWithdrawal().
- Single balances mapping instead of deposits/totalDebited
- Two-step ownership transfer (transferOwnership + acceptOwnership)
- Updated interface, view semantics, and alternatives considered
README covers project overview, contract table, architecture links,
and development instructions. AGENTS.md provides guidelines for AI
agents: repo structure, conventions, domain language, and testing
patterns.
Bring the contract-relevant design context into this repo, which now owns
the contract ADRs (the architecture repo was temporary exploration):
- CONTEXT.md: domain language and architecture map, scoped to the contracts
- ADR-0003: trampoline deployment & settlement integration
- ADR-0004: penalty schedule & attribution (contract-scoped extract)
- ADR-0005: trampoline execution authority & EIP-712 proposal signature
- docs/adr/README.md: index plus known open questions
- docs/reference/: CoW slashing framework, auction mechanics, solver CIPs
- docs/agents/: issue tracker and triage label conventions
Fix the broken references in ADR-0001/0002 (CONTEXT.md, economics note,
old ADR-0006 numbering) and convert code anchors to GitHub permalinks
pinned at cowprotocol/contracts@c6b61ce. The RFP is cited by its forum
URL. AGENTS.md points at CONTEXT.md for vocabulary instead of carrying
its own glossary, and README lists the Trampoline as a planned contract.
Escrow.deposit() gets a TODO for the deploy-at-deposit-time factory hook
settled in ADR-0003; the hook lands with the Trampoline factory.
The reason will be displayed to describe this comment to others. Learn more.
Risk: high. Not approving: this introduces the core Escrow collateral contract and exceeds the medium-risk approval threshold. Cursor Bugbot and Cursor Security Agent were not present; mendesfabio and yvesfracari are already assigned for unresolved review feedback.
Sent by Cursor Approval Agent: Pull Request Router and Approver
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jean-neiverth
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
yvesfracari
left a comment
Summary
Escrowcontract following the spec in ADR-0002, with native-token sub-solver collateral keyed by addresstransferOwnership+acceptOwnership) to prevent irrecoverable typosbalancesmapping for sub-solver accounting — deposits add, debits subtract, withdrawal zeroesrequestWithdrawal→ wait →executeWithdrawaldebit(subSolver, amount, reason)for Track A (revert penalties) and Track B (EBBO passthrough)deposit(subSolver)andwithdrawDebits()(sweeps to owner)balance,effectiveBalance(0 when withdrawal pending),withdrawableBalanceESCROW_OWNER,ESCROW_OPERATOR,COOLDOWN_PERIOD)docs/adr/, escrow ADR updated to reflect implementationTest plan
forge test -vvv)deny = "warnings"lint policyforge fmt --checkpasses