feat: Mock B20Security#37
Conversation
| bytes32 public constant SECURITY_OPERATOR_ROLE = keccak256("SECURITY_OPERATOR_ROLE"); | ||
| bytes32 public constant BURN_FROM_ROLE = keccak256("BURN_FROM_ROLE"); | ||
|
|
||
| bytes32 public constant REDEEMER_SENDER = keccak256("REDEEMER_SENDER"); |
There was a problem hiding this comment.
Flagging we should migrate policy type constants to *_POLICY format for parity with *_ROLE
There was a problem hiding this comment.
Will open a separate PR to fix this
| if (!hasRole(BURN_FROM_ROLE, msg.sender)) { | ||
| revert AccessControlUnauthorizedAccount(msg.sender, BURN_FROM_ROLE); | ||
| } |
There was a problem hiding this comment.
nit: we should probably just have an onlyRole(ROLE) modifier in shared B20 core if not already
There was a problem hiding this comment.
will address in follow on PR
| /// revert is the terminal case. | ||
| function _readPolicyId(bytes32 policyType) internal view virtual override returns (uint64) { | ||
| if (policyType == REDEEMER_SENDER) { | ||
| return uint64(MockB20SecurityStorage.layout().redeemPolicyIds); |
There was a problem hiding this comment.
Callout we forgot to ping on in main channel: we think it's wise to move redeem storage out of B20Security namespace into a new separate Redeemable namespace in case we decide other variants (or core) should also have this functionality. Curious if that also intuits for you?
There was a problem hiding this comment.
happy to hoist since we would want to unify our storage pattern and interfaces accordingly. if we think we're going to hoist redeem though, intuition is to move all redemption-related methods and control to B20 layer.
| // ---------- Redemption ---------- | ||
| // Floor on `redeem` / `redeemWithMemo`, expressed in shares. | ||
| // Defaults to 0 (no floor); admin sets via updateMinimumRedeemable. | ||
| uint256 minimumRedeemable; | ||
| // ---------- Redeem-side policies (PACKED) ---------- | ||
| // Layout: | ||
| // [63:0] redeemerSenderPolicyId | ||
| // [127:64] reserved | ||
| // [191:128] reserved | ||
| // [255:192] reserved | ||
| uint256 redeemPolicyIds; |
There was a problem hiding this comment.
On storage alignment call, we discussed moving redeem functionality to its own B20Redeemable kind of mixin. For now, we only expect securities to implement this, but fundamentally a stablecoin use case could also be reasonable where a user burns stablecoin onchain to receive fiat offchain. Same thing if we consider other variants in the future that involve offchain reserves.
The thought then is to proactively decouple integrating this feature from security storage space so it's easier to share across variants in the future (if needed). Does that feel good to you too?
Co-authored-by: Conner Swenberg <conner.swenberg@coinbase.com>
The mock-b20-security branch introduced MockB20Security.sol and extended MockB20Storage.sol while the main branch concurrently renamed the policy-type identifiers to add a _POLICY suffix (REDEEMER_SENDER -> REDEEMER_SENDER_POLICY, etc.). This commit propagates the rename into the new mock code so the rebased branch builds against the renamed interfaces. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
ilikesymmetry
force-pushed
the
mock-b20-security
branch
from
212ac86 to
695d294
Compare
2 participants
Adds a complete reference implementation for the IB20Security variant under test/lib/mocks/ and, in the process, evolves the interface itself most substantially around
announce, which becomes the canonical "disclosure-and-execute" primitive instead of an unenforced indexer-pairing convention.The interface changes are deliberate ABI/semantic shifts, not just docs: see the announce signature change, new errors, new role, redesigned batchBurn, and tightened redeem below. All changes should be back-ported to the spec doc by EoD.