feat(BOP-495): ERC-8056 interface-review follow-ups (renames + Conversion extension) - #192
Open
robriks wants to merge 10 commits into
Open
Conversation
Rename the ERC-8056 interface file to IERC8056.sol per the Aug 4 2026 B20 interface review. File rename only: the three interface identifiers (IScaledUIAmount, IScaledUIAmountNewUIMultiplier, IScaledUIAmountBalances) and every wire symbol are unchanged. Updates all import paths and one doc comment reference. Co-authored-by: Cursor <cursoragent@cursor.com>
…ors/events Per the Aug 4 2026 B20 interface review, rename the Cobalt-only ERC-8056 scheduled-multiplier symbols to the "UI Multiplier" vocabulary: error ScheduleOverlap -> PendingUpdateExists error NoScheduledMultiplier -> NoScheduledUIMultiplier event MultiplierUpdateCancelled -> UIMultiplierUpdateCancelled Updates the IB20Asset interface, the MockB20Asset reference implementation, the affected unit tests, the scheduled-multiplier smoke journey, and the docs. These symbols are Cobalt-only (AssetV2) and not yet live, so the rename is safe. Co-authored-by: Cursor <cursoragent@cursor.com>
Advertise the canonical instant-failsafe under the ERC-8056 "UI Multiplier" vocabulary as `updateUIMultiplier` and de-advertise the legacy `updateMultiplier` from IB20Asset. The MockB20Asset reference keeps `updateMultiplier` dialable (delegating to the shared body) so the precompile's retained legacy selector stays cross-validated by the fork tests; a regression test pins that the legacy selector remains callable and still emits UIMultiplierUpdated. B20FactoryLib gains encodeUpdateUIMultiplier (canonical) and keeps encodeUpdateMultiplier (legacy, encoded by signature). Docs and smoke journeys are swept separately in the reconciliation commit. Co-authored-by: Cursor <cursoragent@cursor.com>
Add the ERC-8056 Conversion extension interface (id 0x57854fc3) to IERC8056.sol, inherit it on IB20Asset, and advertise it via supportsInterface. The MockB20Asset reference implements toUIAmount/fromUIAmount and keeps the legacy toScaledBalance/toRawBalance dialable (shared bodies) so the precompile's retained legacy selectors stay cross-validated; a regression test pins that they remain callable and agree with the canonical converters. toScaledBalance/toRawBalance are de-advertised from IB20Asset; the dedicated tests move to toUIAmount/fromUIAmount, and B20FactoryLib/docs are updated. The "Conversion extension not claimed" doc statement is flipped. Remaining doc/smoke naming is swept in the reconciliation commit. Co-authored-by: Cursor <cursoragent@cursor.com>
Advertise MAX_UI_MULTIPLIER() (== type(uint128).max) on IB20Asset. The MockB20Asset reference declares it as a public constant (single source of truth, auto-generated getter) and the setter guards reference it, so callers can read the accepted-multiplier bound without hitting the InvalidMultiplier revert path. Pinned to the uint128 overflow guard by a constants test. Co-authored-by: Cursor <cursoragent@cursor.com>
Sweep the docs and smoke journeys for the renamed/added surface and document the
event reconciliation:
- docs/B20/Asset.md: use updateUIMultiplier / toUIAmount / fromUIAmount as the
canonical names (noting the retained-but-de-advertised legacy selectors),
expose MAX_UI_MULTIPLIER(), and spell out the intended double event — an
instant updateUIMultiplier that supersedes a *live* schedule emits
UIMultiplierUpdateCancelled then UIMultiplierUpdated{block.timestamp}, and
AssetV2 never emits the legacy V1 MultiplierUpdated (not part of IB20Asset).
- scheduled_multiplier.py (Cobalt-only): switch to the canonical
updateUIMultiplier / toUIAmount.
- asset_lifecycle.py (cross-fork): keep the legacy updateMultiplier /
toScaledBalance / toRawBalance selectors — the only multiplier/conversion
selectors dialable on both Beryl and Cobalt — with a comment explaining why.
abis.py appends their fragments since IB20Asset no longer advertises them.
Co-authored-by: Cursor <cursoragent@cursor.com>
robriks
requested review from
amiecorso,
eric-ships,
ilikesymmetry,
rayyan224 and
stevieraykatz
as code owners
August 4, 2026 23:01
Interface Coverage✅ All interface functions have test coverage. |
📊 Forge Coverage (
|
| File | Lines | Stmts | Branches | Funcs |
|---|---|---|---|---|
| 🔴 B20FactoryLib.sol | 95.51% | 96.08% | 100.00% | 90.48% |
| 🔴 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.85% | 97.32% | 98.14% | 96.60% |
Full report: download artifact. To browse locally: make coverage (runs forge coverage + genhtml + opens the HTML report).
|
robriks
added a commit
to base/base
that referenced
this pull request
Aug 4, 2026
The Cobalt conformance leg patched current base/base (this PR's renamed/added ERC-8056 surface) into base-anvil but ran the previously-pinned base-std reference, which still expects the pre-rename symbols — a guaranteed mismatch. Point the Cobalt base_std_ref at the paired base-std commit (base/base-std#192) so the two repos are cross-validated in lockstep. Beryl (v1.0.0) is unchanged; V1 is frozen and untouched by this PR. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove verbose comments in IB20Asset.sol, script/smoke/abis.py, and script/smoke/journeys/asset_lifecycle.py per review feedback; the code is self-explanatory. Co-authored-by: Cursor <cursoragent@cursor.com>
robriks
added a commit
to base/base
that referenced
this pull request
Aug 4, 2026
Follow the base-std PR head after dropping review comments (base/base-std#192); keeps the conformance pair in lockstep. Co-authored-by: Cursor <cursoragent@cursor.com>
…MultiplierUpdated Per reviewer intent, the instant multiplier setter (updateUIMultiplier and the retained updateMultiplier) now emits the deprecated MultiplierUpdated(newMultiplier) alongside the ERC-8056 UIMultiplierUpdated, so indexers on the legacy topic keep working. The scheduled setUIMultiplier still emits only UIMultiplierUpdated. Re-declares MultiplierUpdated on IB20Asset, emits it from MockB20Asset's shared instant-setter body (cancel -> MultiplierUpdated -> UIMultiplierUpdated), and updates the affected unit/regression tests, smoke journeys, and docs. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
Author
|
(fork tests need the names in base/base #4285 to land before passing) |
… not removed Per review: match the agreed deprecation pattern (mirroring #193's burnBlocked) — retain the legacy functions in the interface, marked DEPRECATED, rather than removing them. Removing them broke the block-explorer customer class. Re-declares updateMultiplier / toScaledBalance / toRawBalance on IB20Asset with `@notice DEPRECATED.` docs pointing at updateUIMultiplier / toUIAmount / fromUIAmount. Reverts the workarounds the removal forced: drops the manual _DEPRECATED_ASSET_SELECTORS ABI fragments in smoke (the selectors are back in the compiled ABI), restores abi.encodeCall in B20FactoryLib.encodeUpdateMultiplier, and switches the deprecation regression tests to typed calls. Docs/mocks updated. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
|
PR Body still describes the previous approach of dropping deprecated methods. Also your local env is leaking into the PR body via |
Address review feedback (#192): the smoke journeys now assert the instant setter emits the deprecated MultiplierUpdated alongside UIMultiplierUpdated (dual event), so correct the README rows — the `multiplier` journey exercises updateUIMultiplier and expects UIMultiplierUpdated + UIMultiplierUpdateCancelled + MultiplierUpdated; the `asset` rebase emits both events on Cobalt. Also drops a stale example from the assert_no_log docstring. Co-authored-by: Cursor <cursoragent@cursor.com>
stevieraykatz
approved these changes
Aug 5, 2026
ilikesymmetry
left a comment
ilikesymmetry
left a comment
Collaborator
There was a problem hiding this comment.
I'm having second thoughts now on if our setter and cancellation system should be proactively designed for queueing multiple updates. I recall this is a somewhat known use case. If we know we're eventually going to have to build such a thing, I'd prefer to spend an extra cycle to get it right now so we don't have as much to change. Probably worth a longer conversation though.
| error InvalidMetadataKey(); | ||
|
|
||
| /// @notice A multiplier setter (`setUIMultiplier` or `updateMultiplier`) was called with a | ||
| /// @notice A multiplier setter (`setUIMultiplier` or `updateUIMultiplier`) was called with a |
Collaborator
There was a problem hiding this comment.
we only have one setter right?
Suggested change
| /// @notice A multiplier setter (`setUIMultiplier` or `updateUIMultiplier`) was called with a | |
| /// @notice `updateUIMultiplier` was called with a |
| /// | ||
| /// @param pendingEffectiveAt The `effectiveAt` of the live pending update. | ||
| error ScheduleOverlap(uint256 pendingEffectiveAt); | ||
| error PendingUpdateExists(uint256 pendingEffectiveAt); |
Collaborator
There was a problem hiding this comment.
Suggested change
| error PendingUpdateExists(uint256 pendingEffectiveAt); | |
| error UIMultiplierUpdateExists(uint256 effectiveAt); |
|
|
||
| /// @notice `cancelScheduledMultiplier` was called when there is no live pending update | ||
| error NoScheduledMultiplier(); | ||
| error NoScheduledUIMultiplier(); |
Collaborator
There was a problem hiding this comment.
for parity with above error
Suggested change
| error NoScheduledUIMultiplier(); | |
| error UIMultiplierUpdateDoesNotExist(); |
| /// @dev Reverts with `InvalidMultiplier` when `newMultiplier` is zero or above `type(uint128).max`. | ||
| /// | ||
| /// @param newMultiplier New multiplier scaled to `WAD_PRECISION`; must be in `(0, type(uint128).max]`. | ||
| function updateUIMultiplier(uint256 newMultiplier) external; |
Collaborator
There was a problem hiding this comment.
confused, don't we have a setUIMultiplierUpdate that we just need to rename? Whatever function we are adding in Cobalt needs to have the effectiveAt parameter
| /// @dev Reverts with `AccessControlUnauthorizedAccount` when the caller does not hold `OPERATOR_ROLE`. | ||
| /// @dev Reverts with `NoScheduledMultiplier` when there is no live pending update. | ||
| /// @dev Reverts with `NoScheduledUIMultiplier` when there is no live pending update. | ||
| function cancelScheduledMultiplier() external; |
Collaborator
There was a problem hiding this comment.
I think we also wanted this renamed to include "UI" whenever we mention "Multiplier" and keep consistent with "multiplier update" as the noun that is being cancelled theoretically.
Suggested change
| function cancelScheduledMultiplier() external; | |
| function cancelUIMultiplierUpdate() external; |
Comment on lines
41
to
53
| /// @notice `setUIMultiplier` was called with an `effectiveAt` that is not in the future | ||
| /// (`effectiveAt <= block.timestamp`). | ||
| /// | ||
| /// @param effectiveAt Rejected effective-at timestamp. | ||
| error EffectiveAtInPast(uint256 effectiveAt); | ||
|
|
||
| /// @notice `setUIMultiplier` was called with an `effectiveAt` above `type(uint64).max`, the | ||
| /// width of the on-chain `effectiveAt` field. | ||
| /// | ||
| /// @param effectiveAt Rejected effective-at timestamp. | ||
| error EffectiveAtTooFar(uint256 effectiveAt); | ||
|
|
||
| /// @notice `setUIMultiplier` was called while a live pending update already exists |
Collaborator
There was a problem hiding this comment.
something in the prompt seems off to miss all the renames of setUIMultiplier to updateUIMultiplier
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
Applies the Aug 4 2026 B20 Interface Review follow-ups to the ERC-8056 scaled-multiplier surface (Solidity interface + reference mock + tests + smoke). Paired in lockstep with base/base PR base/base#4285 — land together.
Scope is ERC-8056 + multiplier scheduling only. Every wire change is Cobalt-only (AssetV2, not yet activated on any network) or add-alias + deprecate on the frozen Beryl surface — nothing on-chain breaks.
Changes
IScaledUIAmount.sol→IERC8056.sol(file only; interface identifiers unchanged).ScheduleOverlap→PendingUpdateExists,NoScheduledMultiplier→NoScheduledUIMultiplier,MultiplierUpdateCancelled→UIMultiplierUpdateCancelled.updateUIMultiplieris the canonical instant-failsafe.IScaledUIAmountConversion(0x57854fc3) adopted:toUIAmount/fromUIAmountare the canonical converters, advertised viasupportsInterface.MAX_UI_MULTIPLIER()getter exposes thetype(uint128).maxsetter bound.updateUIMultiplier(and the retainedupdateMultiplier) emits both the deprecatedMultiplierUpdated(newMultiplier)and the ERC-8056UIMultiplierUpdated, so indexers on the legacy topic keep working. The scheduledsetUIMultiplieremits onlyUIMultiplierUpdated.Deprecation model (keep in interface, marked deprecated)
Following the team decision (and mirroring #193's
burnBlockedtreatment), the legacy methodsupdateMultiplier/toScaledBalance/toRawBalanceare kept in theIB20Assetinterface, documentedDEPRECATED.— not removed. They remain dialable and aliased under the new names, so block explorers (which need the advertised legacy surface) and developers (who get the canonical names) are both satisfied.Test plan
forge test— all pass (4 fork-gated skips)forge fmt --check, interface-coverage + forge-coverage greenRefs
BOP-495 (parent BOP-429 / B20 Improvements). Paired base/base PR: base/base#4285.