refactor(BOP-440): rename seize surface (seizeWithMemo/SEIZE_ROLE/SEIZE_HOLDER_POLICY); drop burnBlocked changes - #186
Merged
Conversation
…anges Finalizes the seize surface naming and decouples burnBlocked, per the updated BOP-439 decisions (supersedes the #177 design): - Rename across interface + reference impl + tests: - transferFromSeizableWithMemo -> seizeWithMemo - TRANSFER_FROM_SEIZABLE_ROLE -> SEIZE_ROLE - SEIZABLE_ACCOUNT_POLICY -> SEIZE_HOLDER_POLICY - event TransferredFromSeizable -> Seized (role/policy keccak strings change accordingly.) - Drop burnBlockedWithMemo entirely (no burn-with-memo variant). - Revert burnBlocked to its pre-#177 behavior: blocked check on TRANSFER_SENDER_POLICY and the BURN pause vector (no longer part of the seize class). Behavior is otherwise unchanged. - Remove burnBlocked from the IB20 interface as a deprecation signal; the reference impl keeps the selector working for back-compat. Formal deprecation/removal is announced for the Denim hardfork. Retained burnBlocked reference tests call it via a concrete MockB20 cast. SEIZE pause vector now governs only seizeWithMemo. The dedicated seize storage slot (offset 14) is unchanged. All unit tests pass; fmt clean. Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
stephancill
requested review from
amiecorso,
eric-ships,
ilikesymmetry,
rayyan224 and
stevieraykatz
as code owners
July 31, 2026 15:47
Interface Coverage✅ All interface functions have test coverage. |
📊 Forge Coverage (
|
| File | Lines | Stmts | Branches | Funcs |
|---|---|---|---|---|
| 🔴 B20FactoryLib.sol | 95.40% | 96.00% | 100.00% | 90.00% |
| 🔴 test/lib/ForceFeeder.sol | 0.00% | 0.00% | 100.00% | 0.00% |
| 🔴 test/lib/PrecompileProbe.sol | 0.00% | 0.00% | 0.00% | 0.00% |
| 🟢 MockActivationRegistry.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockActivationRegistryStorage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockB20.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockB20Asset.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟡 MockB20Factory.sol | 98.96% | 99.10% | 100.00% | 100.00% |
| 🟢 MockB20Stablecoin.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockB20Storage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟡 MockPolicyRegistry.sol | 100.00% | 99.54% | 97.67% | 100.00% |
| 🟢 MockPolicyRegistryStorage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| Total | 96.75% | 97.26% | 98.09% | 96.46% |
Full report: download artifact. To browse locally: make coverage (runs forge coverage + genhtml + opens the HTML report).
…locked for burnBlocked Splits the shared error so each gate has an accurate name: - seizeWithMemo now reverts AccountNotSeizable(from) (gate: SEIZE_HOLDER_POLICY) - deprecated burnBlocked keeps AccountNotBlocked(from) (gate: TRANSFER_SENDER_POLICY) Membership in SEIZE_HOLDER_POLICY marks who CAN be seized from, so "not blocked" was semantically wrong for the seize path. Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
|
stephancill
commented
Jul 31, 2026
Comment on lines
+311
to
+315
| // Deprecated: `burnBlocked` is retained unchanged for back-compat but is no | ||
| // longer part of the IB20 interface. Its blocked check reads | ||
| // `TRANSFER_SENDER_POLICY` and it gates on the `BURN` pause vector — it is NOT | ||
| // part of the seize operation class (that is `seizeWithMemo`). Formal removal | ||
| // is announced for the Denim hardfork. |
Contributor
Author
There was a problem hiding this comment.
don't need to mention that it is not part of the seize operation class. we should however state that the recommended path is now seize to self and then burn
…then-burn) Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
| // longer part of the IB20 interface. Its blocked check reads | ||
| // `TRANSFER_SENDER_POLICY` and it gates on the `BURN` pause vector. The | ||
| // recommended path is now `seizeWithMemo` to a treasury/self address followed | ||
| // by a normal `burn`. Formal removal is announced for the Denim hardfork. |
Member
There was a problem hiding this comment.
Do we really want to commit in a comment here what code will land in Denim?
| emit Memo(msg.sender, memo); | ||
| } | ||
|
|
||
| // Deprecated: `burnBlocked` is retained unchanged for back-compat but is no |
Member
There was a problem hiding this comment.
Maybe all caps DEPRECATED with a @notice?
Contributor
Author
There was a problem hiding this comment.
AI: Done — converted to a @notice DEPRECATED natspec, with the guard rationale moved to @dev.
Comment on lines
+321
to
+327
| // The point of burnBlocked is to seize from policy-blocked | ||
| // accounts. Read the transfer-sender policy ID out of the | ||
| // transfer-side packed slot and reject if the target is | ||
| // currently authorized. Enforced unconditionally — including | ||
| // for factory-originated calls during the bootstrap window — | ||
| // matching the Rust precompile, which carves no `privileged` | ||
| // exception for this guard. |
Member
There was a problem hiding this comment.
This seems like a weird place for this verbosity. We do one-line comments in impls generally unless something is very complicated. This seems more like natspec body content to me.
Contributor
Author
There was a problem hiding this comment.
AI: Moved the rationale out of the impl body into @dev natspec; the function body no longer carries the block comment.
robriks
self-requested a review
July 31, 2026 18:04
robriks
reviewed
Jul 31, 2026
robriks
left a comment
robriks
left a comment
Collaborator
There was a problem hiding this comment.
aligned with these renames
Comment on lines
+109
to
+110
| /// `SEIZE_HOLDER_POLICY` (i.e. not a member of the seize-holder set, so not eligible to be | ||
| /// seized from). |
Member
There was a problem hiding this comment.
Suggested change
| /// `SEIZE_HOLDER_POLICY` (i.e. not a member of the seize-holder set, so not eligible to be | |
| /// seized from). | |
| /// `SEIZE_HOLDER_POLICY` (i.e. not a member of the seize-holder set). |
|
|
||
| /// @notice Required to call `burnBlocked` and `burnBlockedWithMemo`. | ||
| /// @notice Required to call the deprecated `burnBlocked` (no longer part of this interface; retained for | ||
| /// the back-compat impl, with formal removal announced for Denim). |
Member
There was a problem hiding this comment.
denim called out here too. must have missed first time through
Contributor
Author
There was a problem hiding this comment.
AI: Removed the Denim mention on the BURN_BLOCKED_ROLE getter too. Grep-confirmed there are no remaining Denim references in the .sol files.
… BURN_BLOCKED_ROLE Addresses review: apply the suggested shorter AccountNotSeizable wording, and remove the remaining Denim commitment from the BURN_BLOCKED_ROLE getter. Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
stevieraykatz
approved these changes
Jul 31, 2026
stephancill
added a commit
that referenced
this pull request
Jul 31, 2026
…rney base-std #186 removed burnBlocked from the IB20 interface, so it is absent from STABLECOIN_ABI and web3 raised ABIFunctionNotFound. The selector still dispatches on the precompile (retained for back-compat), so bind it via an explicit fragment to keep exercising the legacy burn-based freeze-and-seize path. Full smoke suite now green (7/7) against a local Cobalt node. Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
stephancill
added a commit
that referenced
this pull request
Jul 31, 2026
…binding) (#187) * test(BOP-486): add transfer-blocked (seize) smoke journey New `seize` smoke journey exercising the Cobalt `seizeWithMemo` surface against a live node: SEIZE_HOLDER_POLICY membership gate + SEIZE_ROLE, the happy-path seizeWithMemo (Transfer -> Memo -> Seized, supply-preserving), the reject gates (AccountNotSeizable, role, InvalidReceiver, ContractPaused), the admin-op decoupling from the receiver policy on `to`, and the independent SEIZE pause vector. Fork-gated: probes the SEIZE_HOLDER_POLICY() getter and skips cleanly on a pre-Cobalt chain, mirroring the multiplier journey. Adds SEIZE_ROLE / SEIZE_HOLDER_POLICY / FEATURE_SEIZE constants to config, registers the journey in the CLI (run order + `all`), and documents it in the README. Complements the stablecoin journey's legacy burn-based burnBlocked path. Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * test(BOP-486): drop burnBlocked note from PausableFeature comment Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * test(BOP-486): rename journey module transfer_blocked -> seize Match the feature/surface name (seizeWithMemo) and the CLI key. The epic is titled Transfer Blocked; the operation is seize. Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * test(BOP-486): drop transfer-blocked wording from seize journey docstring Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * test(BOP-486): bind deprecated burnBlocked selector in stablecoin journey base-std #186 removed burnBlocked from the IB20 interface, so it is absent from STABLECOIN_ABI and web3 raised ABIFunctionNotFound. The selector still dispatches on the precompile (retained for back-compat), so bind it via an explicit fragment to keep exercising the legacy burn-based freeze-and-seize path. Full smoke suite now green (7/7) against a local Cobalt node. Co-authored-by: OpenCode <opencode-noreply@coinbase.com> --------- Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Finalizes the base-std seize surface per the updated BOP-439 decisions, superseding the design merged in #177.
MockB20), and tests:transferFromSeizableWithMemo→seizeWithMemoTRANSFER_FROM_SEIZABLE_ROLE→SEIZE_ROLESEIZABLE_ACCOUNT_POLICY→SEIZE_HOLDER_POLICYTransferredFromSeizable→SeizedburnBlockedWithMemoentirely (no burn-with-memo variant).burnBlockedto its pre-feat(BOP-440): add transferFromSeizableWithMemo and burnBlockedWithMemo seize surface #177 behavior: blocked check onTRANSFER_SENDER_POLICYand theBURNpause vector. It is no longer part of the seize operation class; behavior is otherwise unchanged.burnBlockedfrom theIB20interface as a deprecation signal. The reference impl keeps the selector working for back-compat; formal deprecation/removal is announced for the Denim hardfork. RetainedburnBlockedreference tests call it via a concreteMockB20cast.The
SEIZEpause vector now governs onlyseizeWithMemo. The dedicated seize storage slot (offset 14) is unchanged, so deployed layouts are undisturbed.Testing
forge buildOKforge test— 712 passed, 0 failed, 4 skippedforge fmt --checkcleanSeize suites:
seizeWithMemo(9),seizeWithMemo_revertOrder(5). burnBlocked suites reverted to BURN +TRANSFER_SENDER_POLICY:burnBlocked(7),burnBlocked_revertOrder(6).Comparison vs pre-seize baseline
Diffed against
mainas of the commit right before the seize work (#177) landed, to confirm the net effect is purely the additive seize surface withburnBlockeduntouched. Only #177 plus this branch's commits ever touched the core seize files, so the isolation is clean.burnBlockedeffectively untouched: implementation body is byte-identical to baseline (sameBURNpause,TRANSFER_SENDER_POLICYcheck,AccountNotBlocked,BurnedBlocked). Tests are identical except the call-site casttoken.burnBlocked(...)→MockB20(address(token)).burnBlocked(...), forced only becauseburnBlockedwas removed from theIB20type. Only other addition is a deprecation comment.initializedflag moved 14→15. Offsets 0–13 (every real token-state field) are unchanged, so deployed layouts are undisturbed.initializeddoesn't exist in the Rust precompile, so its relocation is a no-op there. Offset-pin test updated accordingly.SEIZE_ROLE,SEIZE_HOLDER_POLICY,Seizedevent,AccountNotSeizableerror,seizeWithMemo+ getters,_requireSeizable, and the behavior-preserving_moveBalanceextraction.burnBlockedremoved from the interface (intended deprecation), impl retained.burnBlockedWithMemonets to zero vs baseline.PausableFeaturegainsSEIZE(index 3), bumpingALL_FEATURES_PAUSED7→15 (pinned by the removals regression). This is the shared/unversioned enum member that trips the frozen-V1isPaused(3)gotcha on the base/base side.No unexpected changes.