fix(BOP-274): port Rust parity fixes into Solidity mocks #140
fix(BOP-274): port Rust parity fixes into Solidity mocks #140stevieraykatz wants to merge 4 commits into
Conversation
Rust precompile enforces MINT_RECEIVER_POLICY unconditionally on every mint, including factory-originated mints in the bootstrap window. The Solidity mock previously bypassed the check whenever `_isPrivileged()` held, which let initCalls mint to non-authorized accounts. Drop the bypass so the mock matches Rust semantics. An unconfigured slot still reads as ALWAYS_ALLOW_ID, so default-deploy bootstrap flows are unaffected.
…rap (BOP-274) Rust precompile enforces the "target account must be blocked under TRANSFER_SENDER_POLICY" guard unconditionally on burnBlocked. The Solidity mock previously bypassed the check whenever `_isPrivileged()` held. Drop the bypass so factory-originated burnBlocked calls during the bootstrap window also revert AccountNotBlocked unless the target is actually blocked, matching Rust.
…OP-274) The Rust precompile exposes an is_announcement_active runtime flag that flips true for the lifetime of an announce(...) bracket and false at all other times. The Solidity mock did not surface the same view, leaving inner-call contracts no way to detect they were executing inside a disclosed corp action. Add the IB20Asset.isAnnouncementActive() interface entry and back it in MockB20Asset with an EIP-1153 transient bool set at the top of announce and cleared at the bottom. Transient storage auto-clears at tx end, so a revert anywhere in the bracket cannot leave the flag stuck true.
Interface Coverage✅ All interface functions have test coverage. |
📊 Forge Coverage (
|
| File | Lines | Stmts | Branches | Funcs |
|---|---|---|---|---|
| 🟢 B20FactoryLib.sol | 100.00% | 100.00% | 100.00% | 100.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.95% | 99.08% | 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% | 100.00% | 100.00% | 100.00% |
| 🟢 MockPolicyRegistryStorage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| Total | 99.86% | 99.88% | 100.00% | 100.00% |
Full report: download artifact. To browse locally: make coverage (runs forge coverage + genhtml + opens the HTML report).
|
stevieraykatz
changed the title
| /// exposes the same value via a runtime context flag rather | ||
| /// than a storage slot, so there is no persistent layout the | ||
| /// Rust impl needs to mirror. | ||
| bool internal transient _announcementActive; |
There was a problem hiding this comment.
the transient storage location is only available in solidity as of 0.8.28 but our pragma is open to 0.8.20 (not good). we could:
- bump the required version for the whole repo to 0.8.28, or
- use an assembly tstore pattern (supported as of 0.8.24)
- use a regular storage param
I'm partial to bumping the pragma to at least 0.8.24 and using tstorage (more closely reflects the rust revm memory impl), but I could also see us not bringing this feature to parity.
There was a problem hiding this comment.
I think bumping the pragma to support the most expressive/accurate mock implementation makes sense to me? If there were genuine compatibility concerns for other devs importing/working with this source code I'd hesitate more but given that these aren't production implementations I don't think it matters?
| /// | ||
| /// @dev Intended for inner-call contracts dispatched via `internalCalls` (or other | ||
| /// contracts they reach) to detect they are executing inside an announcement | ||
| /// bracket — useful for issuance, multiplier, and metadata flows whose policy |
There was a problem hiding this comment.
I'm not sold on this
There was a problem hiding this comment.
Even if we wanted the state, it's not clear any external consumer would ever be able to use this given internal calls are internal so no external entity gets control flow when announcement is active
There was a problem hiding this comment.
But I'm also not sold the state is even useful given no code is actually using it seemingly?
3 participants
Summary
Brings three Solidity-mock behaviors back into parity with the Rust precompile after Roger flagged the divergences:
_mintno longer skipsMINT_RECEIVER_POLICYwhen called from the factory bootstrap window. Rust enforces the policy unconditionally; the policy itself defaults toALWAYS_ALLOW_IDfor an unconfigured slot, so existing bootstrap flows are unaffected.burnBlockedno longer skips the "target is blocked" check during bootstrap. Same shape as the mint fix: unconditional enforcement, default-allow when unconfigured.isAnnouncementActive()view — adds the missing view toIB20Assetso inner-call contracts can detect they're executing inside anannounce(...)bracket. Backed by an EIP-1153transient boolflipped at the open and close of the bracket; transient storage auto-clears at tx end, so a revert inside the bracket cannot leave the flag stucktrue.Linear: BOP-274. One commit per subtask.
Test plan
forge buildcleanforge test— 610 tests pass (3 new intest/unit/B20Asset/announcement/isAnnouncementActive.t.sol, no regressions)