Skip to content

feat(BOP-495): ERC-8056 interface-review follow-ups (renames + Conversion extension) - #192

Open
robriks wants to merge 12 commits into
mainfrom
markusosterlund/bop-495-schedule-multiplier-updates-interface-review-follow-ups
Open

feat(BOP-495): ERC-8056 interface-review follow-ups (renames + Conversion extension)#192
robriks wants to merge 12 commits into
mainfrom
markusosterlund/bop-495-schedule-multiplier-updates-interface-review-follow-ups

Conversation

@robriks

@robriksrobriks commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

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

  • RenameIScaledUIAmount.solIERC8056.sol (file only; interface identifiers unchanged).
  • Cobalt-only vocabulary:ScheduleOverlapPendingUpdateExists, NoScheduledMultiplierNoScheduledUIMultiplier, MultiplierUpdateCancelledUIMultiplierUpdateCancelled.
  • updateUIMultiplier is the canonical instant-failsafe.
  • IScaledUIAmountConversion (0x57854fc3) adopted: toUIAmount / fromUIAmount are the canonical converters, advertised via supportsInterface.
  • MAX_UI_MULTIPLIER() getter exposes the type(uint128).max setter bound.
  • Dual event on the instant setter:updateUIMultiplier (and the retained updateMultiplier) emits both the deprecated MultiplierUpdated(newMultiplier) and the ERC-8056 UIMultiplierUpdated, so indexers on the legacy topic keep working. The scheduled setUIMultiplier emits only UIMultiplierUpdated.

Deprecation model (keep in interface, marked deprecated)

Following the team decision (and mirroring #193's burnBlocked treatment), the legacy methods updateMultiplier / toScaledBalance / toRawBalance are kept in the IB20Asset interface, documented DEPRECATED. — 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

Refs

BOP-495 (parent BOP-429 / B20 Improvements). Paired base/base PR: base/base#4285.

robriksand others added 6 commits August 4, 2026 16:34
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>
@linear

linearBot commented Aug 4, 2026

Copy link
Copy Markdown

BOP-495

@github-actions

Copy link
Copy Markdown

Interface Coverage

✅ All interface functions have test coverage.

@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown

📊 Forge Coverage (src/lib/)

🟡 ≥95% across all metrics — some metrics below 99%.

FileLinesStmtsBranchesFuncs
🔴 B20FactoryLib.sol95.51%96.08%100.00%90.48%
🔴 test/lib/ForceFeeder.sol0.00%0.00%100.00%0.00%
🔴 test/lib/PrecompileProbe.sol0.00%0.00%0.00%0.00%
🟢 MockActivationRegistry.sol100.00%100.00%100.00%100.00%
🟢 MockActivationRegistryStorage.sol100.00%100.00%100.00%100.00%
🟢 MockB20.sol100.00%100.00%100.00%100.00%
🟢 MockB20Asset.sol100.00%100.00%100.00%100.00%
🟡 MockB20Factory.sol98.96%99.10%100.00%100.00%
🟢 MockB20Stablecoin.sol100.00%100.00%100.00%100.00%
🟢 MockB20Storage.sol100.00%100.00%100.00%100.00%
🟡 MockPolicyRegistry.sol100.00%99.54%97.67%100.00%
🟢 MockPolicyRegistryStorage.sol100.00%100.00%100.00%100.00%
Total96.85%97.32%98.14%96.60%

Full report: download artifact. To browse locally: make coverage (runs forge coverage + genhtml + opens the HTML report).

@github-actions

github-actionsBot commented Aug 4, 2026

Copy link
Copy Markdown

⚠️ Fork tests: 94 failed, 668 passed

These failures indicate divergences where base/base needs to catch up to the base-std spec. This check is advisory and does not block merging.

Failing tests
  • test_cancelScheduledMultiplier_revert_matured(uint256): Error != expected error: custom error 0x002e806f != NoScheduledUIMultiplier(); counterexample: calldata=0x7675e37600000000000000015b593de1b9d8481d015362465380262f5de3ee5a8e788a39 args=[8516962902041904942733119100558150589894627620761034590777 [8.516e57]]
  • test_cancelScheduledMultiplier_success_emitsEvent(uint256,uint256): log != expected log; counterexample: calldata=0xdb3ca1fd0000000000000000000000000000000000001e4c679c0785c0683322c531baa20000000000000001b291d942298a24bd8c4a23b4d935d741822b846a39daad2d args=[614525694044597507529324962626210 [6.145e32], 10655618562647043745361051340511710879947555306852918668589 [1.065e58]]
  • test_cancelScheduledMultiplier_success_leavesCurrentUntouched(uint256): custom error 0xae875db7; counterexample: calldata=0x7f58ca6b00000000000000000000000000000001e8c3bcf35e63ef5bebdfa4be42f9e18f args=[649679592205114619765482915472422330767 [6.496e38]]
  • test_conversion_deprecated_stillDialable(uint256): custom error 0xae875db7; counterexample: calldata=0x5f15495600000000000000000000000000000000000000000000000000000000000018e1 args=[6369]
  • test_createCompositePolicy_revertOrder(uint8): Error != expected error: custom error 0xcbdee0f6: != ChildPoliciesOutsideOfRange(); counterexample: calldata=0x5fafe4f7000000000000000000000000000000000000000000000000000000000000002a args=[42]
  • test_createCompositePolicy_revert_childPoliciesOutsideOfRange(address,address,uint8,uint8): Error != expected error: custom error 0xcbdee0f6: != ChildPoliciesOutsideOfRange(); counterexample: calldata=0xb8466436000000000000000000000000dc3c04910e0885f12756e591374bff6fe4081fd600000000000000000000000016b3baef60aeb16f7b85b61c62b8d6d936958e7a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000b6 args=[0xdC3C04910e0885F12756E591374bff6FE4081Fd6, 0x16B3BaEf60aEB16f7b85B61c62b8d6d936958e7a, 2, 182]
  • test_erc8056Surface_success_aliasesResolve(uint256): custom error 0x3248d4ff; counterexample: calldata=0x91648cee0000000000000000000000000000000000000000000000000000000000000911 args=[2321]
  • test_fromUIAmount_success_explicitZeroMultiplierFallsBackToWad(uint256): custom error 0xae875db7; counterexample: calldata=0x2e5d48fc0000000000000000000000000000000000000000000000000000000000000006 args=[6]
  • test_fromUIAmount_success_identityOnWadDefault(uint256): custom error 0x65cd9b3c; counterexample: calldata=0x143250550000000000000000000000000000000000000000001ec98b7f4d33ef8a468a7f args=[37219543521967039490460287 [3.721e25]]
  • test_fromUIAmount_success_invertsByStoredMultiplier(uint256,uint256): custom error 0xae875db7; counterexample: calldata=0xe11ab12f000000000000000000000000000000000000000000000000000000000000263300000000000000000000000000000000000000000000000000000000c0da474f args=[9779, 3235530575 [3.235e9]]
  • test_fromUIAmount_success_roundTripExactOnWadDefault(uint256): custom error 0x3248d4ff; counterexample: calldata=0x76c2c36b000000000000000000000000000000000000000000000000fe73195c72643f6b args=[18335026392802279275 [1.833e19]]
  • test_fromUIAmount_success_roundTripFloors(uint256,uint256): custom error 0xae875db7; counterexample: calldata=0x3f088b7a00000000000e18af1dd72846e3ebc61543753eed1e45c7d559a1b9b8ffeae4e30000000000000000000000000000000000000000000000000000000000000ff7 args=[5798931688176038758526599480743477830829080826904139679505507555 [5.798e63], 4087]
  • test_fromUIAmount_success_zeroUIAmount(uint256): custom error 0xae875db7; counterexample: calldata=0x1da822730000000000000000000000000093c347405ed49e65b8a11a78ae4f127c13b875 args=[3295220685551726291836478348980172286128797813 [3.295e45]]
  • test_multiplierEvent_success_emitsUIAndDeprecated(uint256): custom error 0xae875db7; counterexample: calldata=0x3382e24d0000000000000000000000000000000000000000000000000000000000001000 args=[4096]
  • test_multiplier_success_renamedFromShareRatio(uint256): custom error 0x3248d4ff; counterexample: calldata=0x94be8dcc00000000000000000000000000000000000000000000000000000000000002a9 args=[681]
  • test_multiplier_success_returnsStoredValue(uint256): custom error 0xae875db7; counterexample: calldata=0x538a9cdfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff args=[115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]]
  • test_scaledBalanceOf_success_explicitZeroMultiplierFallsBackToWad(address,uint256): custom error 0xae875db7; counterexample: calldata=0xd7c2e926000000000000000000000000cb000000000000000000000000000000000000000000000000000000000000000000000000000000000000000de0b6b3a7640000 args=[0xCB00000000000000000000000000000000000000, 1000000000000000000 [1e18]]
  • test_scaledBalanceOf_success_scalesByStoredMultiplier(address,uint256,uint256): custom error 0xae875db7; counterexample: calldata=0x6a8c4733000000000000000000000000521e035d9f1c76a88eddba7d8dd0178a5f462d7000000000000000000000000000000000000000000000000000000014b9a3ea35000000000000000000000000000000000000000000004d66a1ee21486ea4eb30 args=[0x521E035d9f1C76a88EDDba7D8dd0178a5F462D70, 89013873205 [8.901e10], 365515455376760596917040 [3.655e23]]
  • test_scaledBalanceOf_success_zeroForEmptyAccount(address,uint256): custom error 0xae875db7; counterexample: calldata=0x2f24150f0000000000000000000000008d7b34a50c91664c91eaf78a739ac89a61e0e78000000000000000000000000068de1fdb7ad4c345cc97a1fc00924ee555374470 args=[0x8D7b34A50c91664C91eAF78A739AC89a61e0e780, 598688580765725892130540736830867043010251015280 [5.986e47]]
  • test_setUIMultiplier_revert_pendingUpdateExists(uint256,uint256): Error != expected error: custom error 0x91a52665: 000000000000000000000000000000000000000000000000214e3a9d993b8143 != PendingUpdateExists(2399920099990143299 [2.399e18]); counterexample: calldata=0xbef277f1000000000000000000000000000000000000000000000004214e3a9bef6c4aef00000000000000000000000001cdffc8bee3bcb6ef9ae0b41134a9a61319b114 args=[76186896387684453103 [7.618e19], 10302925479644398491542186781802457779540701460 [1.03e46]]
  • test_toUIAmount_revert_arithmeticOverflow(uint256,uint256): custom error 0xae875db7; counterexample: calldata=0xabfa2de9000000000000000000000000000000000000000000000000000000000000000200000000000000000000000063b079ef36ba08dbb117d769c264ccf33da5e44b args=[2, 569125639453329198319253563761261450848490611787 [5.691e47]]
  • test_toUIAmount_success_explicitZeroMultiplierFallsBackToWad(uint256): custom error 0xae875db7; counterexample: calldata=0x800dbd6f0000000000000000000000000000000000000000000000000000000057854fc3 args=[1468354499 [1.468e9]]
  • test_toUIAmount_success_identityOnWadDefault(uint256): custom error 0x3248d4ff; counterexample: calldata=0x199e403a0000000000000000000000000000000000000000cf7db03a995a68303768ff3e args=[64215344977932658397583834942 [6.421e28]]
  • test_toUIAmount_success_scalesByStoredMultiplier(uint256,uint256): custom error 0xae875db7; counterexample: calldata=0xa378d21900000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000cef args=[16, 3311]
  • test_toUIAmount_success_zeroRawAmount(uint256): custom error 0xae875db7; counterexample: calldata=0x3f144b34fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffc args=[115792089237316195423570985008687907853269984665640564039457584007913129639932 [1.157e77]]
  • test_totalSupplyUI_success_scalesByMultiplier(uint256,uint256): custom error 0xae875db7; counterexample: calldata=0x5c654382000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000d505acd0 args=[128, 3573918928 [3.573e9]]
  • test_totalSupplyUI_success_zeroWhenNoSupply(uint256): custom error 0xae875db7; counterexample: calldata=0x605620a900000000000000000000000000000000000000000067f8981397bf292f3db138 args=[125693311624964872670654776 [1.256e26]]
  • test_updateComposite_revertOrder(uint8): Error != expected error: custom error 0xcbdee0f6: != ChildPoliciesOutsideOfRange(); counterexample: calldata=0x46b240f90000000000000000000000000000000000000000000000000000000000000002 args=[2]
  • test_updateComposite_revert_childPoliciesOutsideOfRange(uint8,uint8): Error != expected error: custom error 0xcbdee0f6: != ChildPoliciesOutsideOfRange(); counterexample: calldata=0x25c44760000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000a1 args=[1, 161]
  • test_updateMultiplier_deprecated_stillDialable(uint256): deprecated updateMultiplier must also emit MultiplierUpdated: -1 <= -1; counterexample: calldata=0x8e38ffd6000000000000000000000000000000000000000000000000000000000000213c args=[8508]
  • test_updateUIMultiplier_revertOrder(address): Error != expected error: custom error 0xae875db7 != AccessControlUnauthorizedAccount(0xCB00000000000000000000000000000000000000, 0x97667070c54ef182b0f5858b034beac1b6f3089aa2d3188bb1e8929f4fa9b929); counterexample: calldata=0x1c8f3921000000000000000000000000cb00000000000000000000000000000000000000 args=[0xCB00000000000000000000000000000000000000]
  • test_updateUIMultiplier_revert_aboveUint128Ceiling(uint256): Error != expected error: custom error 0xae875db7 != InvalidMultiplier(); counterexample: calldata=0x83cbf42dcdcc772fe4cbdb1029f822861176d09e646db96723d4c1e82ddfdeb8163ef54c args=[93085393359812225330546714545801767655967775857010822972004765400649682580812 [9.308e76]]
  • test_updateUIMultiplier_revert_metadataRoleInsufficient(uint256): Error != expected error: custom error 0xae875db7 != AccessControlUnauthorizedAccount(0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e, 0x97667070c54ef182b0f5858b034beac1b6f3089aa2d3188bb1e8929f4fa9b929); counterexample: calldata=0x8e55eaf300000000000000000000000000000000000000000000000000000000001e8480 args=[2000000 [2e6]]
  • test_updateUIMultiplier_revert_unauthorized(address,uint256): Error != expected error: custom error 0xae875db7 != AccessControlUnauthorizedAccount(0x0000000000000000000000000000000000001955, 0x97667070c54ef182b0f5858b034beac1b6f3089aa2d3188bb1e8929f4fa9b929); counterexample: calldata=0x3b75c88400000000000000000000000000000000000000000000000000000000000019550000000000000000000000000000000000000000000000000000000000000ddf args=[0x0000000000000000000000000000000000001955, 3551]
  • test_updateUIMultiplier_success_emitsEvent(uint256): log != expected log; counterexample: calldata=0xbf92144d000000000000000000000000000000000000000000000000fc318e6e81bf35eb args=[18172462576691000811 [1.817e19]]
  • test_updateUIMultiplier_success_writesSlot(uint256): custom error 0xae875db7; counterexample: calldata=0x7d1ae207000000000000000000000000000000000000000000000000000000000001ff48 args=[130888 [1.308e5]]
    [FAIL: Error != expected error: custom error 0x002e806f != NoScheduledUIMultiplier()] test_cancelScheduledMultiplier_revert_noPending() (gas: 48055)
    [FAIL: Error != expected error: custom error 0xae875db7 != InvalidMultiplier()] test_updateUIMultiplier_revert_zeroMultiplier() (gas: 45752)
    [FAIL: custom error 0x54309870] test_maxCompositeChildPolicies_success() (gas: 2586)
    [FAIL: custom error 0x785c0cf0] test_maxUIMultiplier_success_equalsUint128Max() (gas: 5192)
    [FAIL: custom error 0xae875db7] test_announce_innerPanic_propagatesRaw() (gas: 47500)
    [FAIL: custom error 0xae875db7] test_b20AssetLayout_success_populatedSnapshotMatchesAllSlots() (gas: 45393)
    [FAIL: custom error 0xae875db7] test_multiplier_success_zeroRestoresWadFallback() (gas: 45345)
    [FAIL: custom error 0xae875db7] test_updateUIMultiplier_success_clearsMaturedPendingNoCancelEvent() (gas: 74843)
    [FAIL: custom error 0xb3ae29f7] test_minCompositeChildPolicies_success() (gas: 2519)
    [FAIL: log != expected log] test_updateUIMultiplier_success_clearsLivePending() (gas: 80712)
    [FAIL: must advertise IScaledUIAmountConversion] test_supportsInterface_success_claimedIds() (gas: 11895)

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>
@robriksrobriks self-assigned this Aug 5, 2026
@robriks

Copy link
Copy Markdown
CollaboratorAuthor

(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>
@stevieraykatz

stevieraykatz commented Aug 5, 2026

Copy link
Copy Markdown
Member

PR Body still describes the previous approach of dropping deprecated methods. Also your local env is leaking into the PR body via ~/work/erc8056... in the Refs section.

Comment threadscript/smoke/README.md Outdated
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>

@ilikesymmetryilikesymmetry left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment threadsrc/interfaces/IB20Asset.sol Outdated
error InvalidMetadataKey();

/// @notice A multiplier setter (`setUIMultiplier` or `updateMultiplier`) was called with a
/// @notice A multiplier setter (`setUIMultiplier` or `updateUIMultiplier`) was called with a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are two, the instantaneous failsafe (legacy) updateMultiplier and the newer updateUIMultiplier

Comment threadsrc/interfaces/IB20Asset.sol Outdated
Comment threadsrc/interfaces/IB20Asset.sol Outdated
Comment threadsrc/interfaces/IB20Asset.sol Outdated
/// @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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we (b20 + tequities) had decided to keep the instantaneous updateMultiplier function as an admin failsafe, partly because its "tech debt" can't be aliased under the new function

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as opposed to the new updateUIMultiplier that accepts the effectiveAt parameter)

Comment threadsrc/interfaces/IB20Asset.sol Outdated
Comment threadsrc/interfaces/IB20Asset.sol Outdated
…ultiplier vocabulary for cancel + errors
Addresses ilikesymmetry's review on #192. The canonical Cobalt setter is now the
scheduled `updateUIMultiplier(newMultiplier, effectiveAt)` (rename of `setUIMultiplier`),
rather than an added instant alias. The instant path stays as the deprecated
`updateMultiplier` (retained, dialable). Also, for "UI multiplier update" vocabulary parity:
- cancelScheduledMultiplier -> cancelUIMultiplierUpdate
- PendingUpdateExists(effectiveAt) -> UIMultiplierUpdateExists(effectiveAt)
- NoScheduledUIMultiplier -> UIMultiplierUpdateDoesNotExist
The redundant instant `updateUIMultiplier(uint256)` alias is dropped. Interface, mock,
factory encoders, unit/regression/storage tests, smoke journeys and docs updated in lockstep.
Co-authored-by: Cursor <cursoragent@cursor.com>
@robriks

Copy link
Copy Markdown
CollaboratorAuthor

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.

this was litigated in a conversation with the tequities team around 3 weeks ago, and the current design leaves room for future extension.

ofc we can reverse that decision but I toward that being overengineering for a problem that we have not faced yet. it would also bring into question the value of the ongoing audit since a refactor of that size touches more than just the cosmetic naming we're currently refactoring

…95-schedule-multiplier-updates-interface-review-follow-ups
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@robriks@stevieraykatz@stephancill@ilikesymmetry