Repository navigation
[StepSecurity] Apply security best practices - #88
Closed
stepsecurity-app[bot] wants to merge 94 commits into
Closed
stepsecurity-app[bot] wants to merge 94 commits into
stepsecurity-app[bot] wants to merge 94 commits into
Conversation
IDefaultToken is the base interface every B-20 token implements (Default, Stablecoin, Asset all inherit). ERC-20 selector compat, plus memo siblings, mint/burn, role-based access control, pause, EIP-2612 + ERC-1271 permit, transfer-policy hookup, supply cap, and ERC-7572 contractURI. Capabilities is a small library of immutable feature bits that gate the optional surface of any B-20 token. Functions whose capability bit is unset revert with FeatureDisabled, regardless of role state. This is the honest-signaling mechanism: integrators read capabilities() once and know exactly what is permanently possible on the token. Includes named presets ALL, IMMUTABLE_MEMECOIN, and FIXED_SUPPLY.
IStablecoin extends IDefaultToken with a single addition: an immutable currency() identifier (USD, EUR, BTC, etc.) for DEX/routing/wallet categorization. Other stablecoin-specific extensions (reserve attestation, master-minter, freeze, yield distribution) deferred pending team alignment and CDP Custom Stablecoin reference access. IAssetToken extends IDefaultToken with the Tangor-derived asset surface: announcement coupling for metadata changes, share ratio for split-safe accounting, compliant create() with per-caller rate limit, user-side redeem() gated on a separate redeemPolicyId allowlist, batch adminMint/adminBurn cold paths, multi-key extra metadatas, name and symbol updates with announcement coupling. Capabilities extended with asset-specific bits in the 16+ range (ASSET_CREATABLE, ASSET_REDEEMABLE, SHARE_RATIO_MUTABLE, ASSET_METADATA_MUTABLE, ASSET_ADMIN_BATCH) and a STANDARD_EQUITY preset that captures the typical asset-token configuration (inherited mint/burn off, asset paths on, BURN_BLOCKED on for sanctions enforcement). Open questions and unilateral assumptions documented in DESIGN_NOTES.md (separate commit).
Catalogs every decision I made unilaterally vs. every question that needs your input, organized by file and topic. Categories: - ASSUMED: decisions made without explicit sign-off, flag any to revisit - OPEN: questions awaiting your input - VERIFY: ambiguities in source docs (user stories vs. wiki vs. Tangor) Includes a top-9 list of bits to explicitly confirm before iterating further.
…ablecoin After reading coinbase/custom-stablecoin (CCS) end-to-end, the following changes: IDefaultToken: - Split ISSUER_ROLE into MINT_ROLE and BURN_ROLE so issuance and destruction authority can be held separately. Same pattern as CCS; enables operational separation of concerns (treasury team mints, redemption team burns). - Add two-step admin transfer with configurable delay (the OZ AccessControlDefaultAdminRules pattern, also used by CCS): adds defaultAdmin, pendingDefaultAdmin, defaultAdminDelay, plus the begin/cancel/accept/changeDelay/rollbackDelay flow. grantRole and revokeRole now revert when called for DEFAULT_ADMIN_ROLE; the only valid transfer path is the two-step flow with delay. Protects against typo-bricking the admin role and gives detection time on key compromise. IStablecoin: - Add per-minter rate limiting: configureMinter, removeMinterRateLimit, grantMinterRoleWithLimit (atomic combo to avoid first-mint race), currentMintLimit, mintRateLimitConfig. Held under MINT_RATE_LIMIT_ROLE separately from DEFAULT_ADMIN_ROLE. - Add ERC-3009 transfer-with-authorization: transferWithAuthorization, receiveWithAuthorization (front-run-resistant; only payee submits), cancelAuthorization. Both ECDSA (canonical) and bytes (EOA + ERC-1271) variants. Distinct from EIP-2612 permit (random nonces, time windows, direct transfers vs. allowances). - Currency identifier kept from prior draft. Capabilities: - New stablecoin bits in 24..31 range: STABLECOIN_MINT_RATE_LIMITED and STABLECOIN_AUTHORIZATIONS. - New STANDARD_STABLECOIN preset that includes rate limiting + authorizations, OMITS BURN_BLOCKED to default to the CCS 'freeze, never seize' philosophy. Issuers wanting force-burn for sanctions enforcement OR in BURN_BLOCKED at creation.
…decisions Restructure DESIGN_NOTES.md to lead with a Design Rationale section that captures the reasoning behind the major settled decisions, so future contributors do not have to re-derive them: Cross-cutting: - Capabilities bitfield as honest-signaling mechanism - Default IS Core (variant inheritance, no separate ICoreToken) - Single source of truth for compliance (external Policy Registry vs internal blocklist), with note on Tangor sanctions vs CCS freeze - Memos as ERC-20-compatible sibling functions (vs optional parameter) - No third-party deps constraint and its implications Roles & Admin: - Why MINT_ROLE and BURN_ROLE are separate - Why PAUSE_ROLE and UNPAUSE_ROLE are separate - Why two-step admin transfer with delay (typo-bricking protection, key-compromise detection window) - User-defined role support Stablecoin-specific: - Why per-minter rate limiting (risk management, multi-party governance) - Why ERC-3009 (USDC parity for payment apps; complementary to permit) - Currency identifier convention - Freeze vs seize philosophy (CCS vs Tangor) expressed via BURN_BLOCKED Asset-specific: - Three issuance paths (create / adminMint / inherited mint) - redeemPolicyId separate from transferPolicyId - Why on-chain announcement coupling (vs off-chain audit policy) - Share ratio for split-safe accounting (DeFi composability) Open questions reorganized to mark resolved items as RESOLVED with context, surface the remaining 9 items in a confirmation summary at the bottom.
Three policy types in v1: WHITELIST, BLACKLIST, COMPOUND. Built-in IDs 0 (always-reject) and 1 (always-allow) reserved as well-known protocol constants (no view-function getters; documented in the contract notice). Surface mirrors Tempo TIP-403 + TIP-1015 with three deliberate omissions: no virtual-address rejection (no TIP-1022 on Base), no receive policies (no TIP-1028 escrow), no callback / richer-guard policies (deferred; can be added in a future hardfork as a backward- compatible enum extension). Authorization queries split per role (isAuthorizedSender / isAuthorizedRecipient / isAuthorizedMintRecipient) so compound policies can carry asymmetric rules; isAuthorized retained as the sender-AND-recipient composite for callers that want a single check. Compound policies are structurally immutable (constituent IDs cannot change after creation); to rotate the configuration, create a new compound policy and re-point the consuming token's transferPolicyId. Constituents may be simple policies or built-ins (0, 1) but never other compound policies (PolicyNotSimple). DESIGN_NOTES updated with the rationale for shipping just Levels 1+2 (not Level 3 callback policies or Level 4 modular guards) in v1, and which rule classes that decision leaves unsupported chain-side (per-tx amount limits, counterparty-dependent rules).
Singleton factory at a fixed precompile address. Three permissionless create methods (createDefault, createStablecoin, createAsset), each accepting a variant-specific params struct. Tokens are deployed at deterministic addresses derived from (variant, creator, salt). Variant is encoded in the address prefix so the variant of any token is recoverable via variantOf without an SLOAD; address prediction functions (predict*Address) let callers compute the address from (creator, salt) alone, independent of the other creation params. Default and Stablecoin variants accept an initialSupply minted atomically at creation; asset tokens have no initialSupply and bootstrap via create() / adminMint() after deployment. The bootstrap mint deliberately bypasses both the policy check and the MINTABLE capability gate (one-shot creator-managed allocation; rationale captured in DESIGN_NOTES). Each token gets a per-token defaultAdminDelay configured at creation so different asset postures (stablecoin vs memecoin) can have appropriately different delays from day one. Factory is permissionless and has no admin; each created token self-governs via its own roles.
feat(interfaces): first-pass design for the three B-20 token variants
Apply PRD DEFAULT row decisions across token interfaces
IDefaultToken → IB20, IAssetToken → IB20Asset, IStablecoin → IB20Stablecoin. Updates all imports, inheritance declarations, and natspec references across Capabilities.sol, PauseVectors.sol, ITokenFactory.sol, and DESIGN_NOTES.md. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
feat(interfaces): rename token interfaces to IB20 naming scheme
Major simplification of the policy registry interface per the finalized PRD decisions: Type system: - WHITELIST/BLACKLIST -> ALLOWLIST/BLOCKLIST (terminology) - COMPOUND removed entirely. Per-role asymmetric authorization moves to the token layer (multiple policy IDs per token, one per role slot); the registry stops at flat membership checks. Authorization: - isAuthorizedSender/Recipient/MintRecipient -> single isAuthorized. With no on-registry composition, role distinctions have no meaning here; tokens consult the registry per-role using different policy IDs. Membership updates (batch only): - modifyPolicyWhitelist/Blacklist -> updateAllowlist/Blocklist taking address[] of accounts plus a single bool. Single-account update is expressed as a batch of one. Emits a single event per batch. Admin model (2-step + renounce): - setPolicyAdmin (single-step) -> stageUpdateAdmin + finalizeUpdateAdmin. Two-step transfer defends against typos and key compromise: the staged candidate must actively claim the role. - New renounceAdmin (single-step) permanently freezes the policy's member set by clearing the admin slot. After renunciation the policy still answers isAuthorized but cannot be mutated. - Pending admin lives in its own mapping; pendingPolicyAdmin view exposes the in-flight state. Read surface: - policyData() (combined) -> policyType() + policyAdmin() (separate). - New pendingPolicyAdmin() view. - policyIdCounter -> nextPolicyId (clearer naming for 'the next ID that will be assigned'). Dropped from interface entirely (no longer relevant): - PolicyData struct, CompoundPolicyData struct - createCompoundPolicy, compoundPolicyData - PolicyNotSimple error, CompoundPolicyCreated event - All compound natspec Documented as future-deferred (not in v1): - Union/intersect policies (path: append PolicyType enum values + add createUnionPolicy/createIntersectPolicy creators in a future hardfork; backward-compatible since enum extension preserves existing values).
IB20 (default token):
- Add BURN_BLOCKED_ROLE + burnBlocked(from, amount) for sanctions
seizure. Function requires the target to be NOT authorized under the
active TRANSFER_SENDER policy (i.e. on the sender-side blocklist);
rejects with AccountNotBlocked(from) otherwise. Emits Transfer plus
a distinct BurnedBlocked(caller, from, amount) event so indexers can
distinguish compliance seizure from regular burn. Tokens following
'freeze, never seize' simply never grant the role.
- Replace explicit transferPolicyId / changeTransferPolicyId with a
generic policy system: policyId(bytes32) + updatePolicy(bytes32, uint64).
Five standard policy-type constants (keccak256-hashed names per OZ
AccessControl convention):
TRANSFER_SENDER — checked against from on every transfer
TRANSFER_RECEIVER — checked against to on every transfer
TRANSFER_EXECUTOR — checked against msg.sender on transferFrom
when distinct from from
MINT_RECEIVER — checked against to on every mint
REDEEMER_SENDER — checked against msg.sender on redeem (used by
variants that ship redeem, e.g. IB20Asset)
Asymmetric per-role configuration is now token-side (multiple policy
IDs, one per slot) rather than registry-side (compound policies).
All slots default to ID 0 (always-reject); tokens cannot move balance
until admin configures their compliance regime.
- Remove redeem surface (redeem, redeemWithMemo, minimumRedeemable,
setMinimumRedeemable, Redeemed event, MinimumRedeemableUpdated
event, MinimumRedeemableNotMet error) — relocated to IB20Asset
where it belongs semantically. The REDEEMER_SENDER policy-type
constant stays on IB20 so all B-20 tokens share a common policy-type
vocabulary.
- Replace TransferPolicyUpdated event with generic
PolicyUpdated(bytes32 policyType, uint64 oldPolicyId, uint64 newPolicyId).
- Update transfer / transferFrom / mint natspec to describe per-slot
policy checks (TRANSFER_SENDER on from, TRANSFER_RECEIVER on to,
TRANSFER_EXECUTOR on msg.sender when distinct from from,
MINT_RECEIVER on mint to).
- Refine PolicyForbids error to carry both the policy type (which slot)
and the policy ID (which registry entry).
IB20Asset:
- Host the full redeem surface (redeem, redeemWithMemo, minimumRedeemable,
setMinimumRedeemable, Redeemed event, MinimumRedeemableUpdated event,
MinimumRedeemableNotMet error). Redeem now uses the inherited
REDEEMER_SENDER policy slot from IB20's generic policy system
(rather than the compound-policy redeemer slot we previously
documented). Tokens that do not offer redemption point that slot at
policy ID 0 (always-reject).
- Refresh natspec to reflect the inherited generic policy system: the
brokerage allowlist is just the policy at REDEEMER_SENDER, configured
by the admin via updatePolicy. No separate redeemPolicyId field on
the asset token; it is a normal policy slot in the generic mapping.
- Document the role separation more explicitly: 'asset issuers
typically do not grant MINT_ROLE; create / adminMint are the
canonical issuance paths' and analogous notes for BURN_ROLE.
REDEEMER_SENDER is only consulted by the redeem path, which lives on IB20Asset. Exposing it on the base IB20 surface implied all B-20 tokens share that vocabulary, but in practice nothing on Default ever references the slot. Move the constant to where the function that uses it lives. The underlying policyId(bytes32) mapping on IB20 still accepts any bytes32 key, so this is a pure interface relocation: no change to the generic policy storage shape, no impact on how the registry is called, no breaking change for callers that read the slot via the inherited generic accessor.
Sync IB20 + IPolicyRegistry interfaces with finalized PRD
Reshape ITokenFactory to a single createToken(variant, salt, params,
initCalls) entry point with one TokenCreated event and one
getTokenAddress predictor, dropping the three variant-specific
creators. Params become minimal version-prefixed structs; optional
post-creation state (initial supply, supply cap, policy slot
assignments, capabilities, contract URI, etc.) moves to initCalls so
the factory can stay immutable while variants evolve via new version
bytes. Document the fixed factory address (0xB20...000F) and the
20-byte token address schema, including decimals-in-address byte for
stateless decimals() lookup.
Flip IPolicyRegistry built-ins: ID 0 now means always-allow ("no
policy on this slot") and type(uint64).max means always-reject.
nextPolicyId starts at 1. Update IB20 + IB20Asset doc references
so newly created tokens are unrestricted by default rather than
inert-until-configured, matching the spec's "absence of policy = no
restriction" framing.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Align with the `getTokenAddress` naming convention for the factory's view surface. Signature unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Collapse factory surface and flip policy built-in IDs per PRD
Expose the TokenFactory (0xB20...000F) and PolicyRegistry (0xb00...001) singleton addresses as named constants alongside interface-typed handles, so callers can invoke them directly (e.g. `StdPrecompiles.TOKEN_FACTORY.createToken(...)`) without re-deriving the address or re-casting the handle at each call site. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Add StdPrecompiles library for singleton precompile handles
Move PauseVectors from src/interfaces/ into src/utils/, where it better belongs as a helper library rather than part of the interface surface. Replace the four bit constants (MINT, BURN, TRANSFER, REDEEM) with a `PausableFeature` enum and a single `toPauseVector` helper that OR's a feature set into the uint256 bitmask `IB20.pause` expects. Delete src/interfaces/Capabilities.sol entirely: the capabilities concept is no longer needed. Scrub the corresponding IB20 surface (`capabilities()`, `isPausable()`, `isCapMutable()`, `FeatureDisabled` error, and all "requires X capability" doc fragments on `pause`, `unpause`, and `setSupplyCap`) along with the IB20Asset and ITokenFactory doc references to capability bits. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Consolidated PR rebased onto current main. Squashed from 6 commits;
preserves the helpers (`_creatablePolicyType`, `_boundAccounts`) and
the `_firstLogIndex` log-ordering helper introduced on main.
Changes:
- Rename `ITokenFactory.isInitialized` → `isB20Initialized`.
- Strip view-function reverts from PolicyRegistry queries (`isAuthorized`,
`policyExists`, `policyAdmin`, `pendingPolicyAdmin`) — malformed and
uncreated IDs now return the "absent" value. Mutating entry points
keep their `PolicyNotFound` reverts.
- Simplify `PolicyType` to `{BLOCKLIST = 0, ALLOWLIST = 1}`. Remove
`policyType()` view and `InvalidPolicyType` / `MalformedPolicyId`
errors (type recoverable from ID alone).
- Sentinel IDs: `ALWAYS_ALLOW_ID = 0`; `ALWAYS_BLOCK_ID =
(uint64(ALLOWLIST) << 56) | 1`. Each reads naturally as the
degenerate form of its list type (empty BLOCKLIST → allow all;
empty ALLOWLIST → block all).
- Packed policy slot drops the type byte and adds an exists flag at
bit 255: `[255] exists | [254:160] unused | [159:0] admin`. The
flag survives `renounceAdmin` so `packed == 0` still uniquely
means "never created".
- Drop external `isMember` and `nextPolicyId`. Tests that need to
predict the next ID use a new `_predictNextPolicyId` test helper
that reads `nextCounter` directly via `vm.load`.
- `isAuthorized` hot path: one SLOAD (the membership bit). No
existence check — `IB20.updatePolicy` validates `policyExists` at
write time, so the on-token policy slot can never reference a
non-existent ID. For non-existent IDs the query collapses to
empty-member-set semantics.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* script: activate B20Stablecoin and B20Asset features Added in base/base PR #2806 (Stablecoin) and PR #2813 (Asset). Required after pulling base/base@d7662c05e for the full B-20 variant suite to dispatch. * fork-testing: switch from base-anvil wrapper to stock anvil --base The base-anvil fork's wrapper binary (--base-precompiles) was deleted upstream in favor of the --base flag flattened into NetworkConfigs, which stock anvil and forge already pick up. Rename ANVIL_BIN env var, default binary paths, log file, and all references in script/run-fork-tests.sh; update FORK_TESTING.md prose accordingly. Verified end-to-end: script launches anvil + forge from ~/code/base-anvil/target/release/, activates all 5 features, and runs test_transfer_success_debitsSender to PASS against live Rust precompile dispatch. * fork-testing: drop required base/base sibling clone, document bump-base.sh base-anvil now pulls base/base as a git dep instead of a path dep, so new users no longer need to maintain a sibling base/base clone just to build. Update prerequisites (two repos instead of three) and the precompile-update workflow to use ./script/bump-base.sh in base-anvil instead of `cd base && git pull && rebuild`. Adds a "Local-iteration override" section documenting the [patch."https://github.com/base/base.git"] block for the cases where a local clone IS needed (unpushed branches, applying unmerged patches). Adds the dispatch.rs B20_STABLECOIN failure mode + workaround to the common-failures section. Updates the "where everything lives" table to reflect the new pin location (networks/Cargo.toml) and the override location (workspace Cargo.toml).
* script: activate B20Stablecoin and B20Asset features Added in base/base PR #2806 (Stablecoin) and PR #2813 (Asset). Required after pulling base/base@d7662c05e for the full B-20 variant suite to dispatch. * fork-testing: switch from base-anvil wrapper to stock anvil --base The base-anvil fork's wrapper binary (--base-precompiles) was deleted upstream in favor of the --base flag flattened into NetworkConfigs, which stock anvil and forge already pick up. Rename ANVIL_BIN env var, default binary paths, log file, and all references in script/run-fork-tests.sh; update FORK_TESTING.md prose accordingly. Verified end-to-end: script launches anvil + forge from ~/code/base-anvil/target/release/, activates all 5 features, and runs test_transfer_success_debitsSender to PASS against live Rust precompile dispatch. * fork-testing: drop required base/base sibling clone, document bump-base.sh base-anvil now pulls base/base as a git dep instead of a path dep, so new users no longer need to maintain a sibling base/base clone just to build. Update prerequisites (two repos instead of three) and the precompile-update workflow to use ./script/bump-base.sh in base-anvil instead of `cd base && git pull && rebuild`. Adds a "Local-iteration override" section documenting the [patch."https://github.com/base/base.git"] block for the cases where a local clone IS needed (unpushed branches, applying unmerged patches). Adds the dispatch.rs B20_STABLECOIN failure mode + workaround to the common-failures section. Updates the "where everything lives" table to reflect the new pin location (networks/Cargo.toml) and the override location (workspace Cargo.toml). * fork-testing: document stock foundry prereq for cast run-fork-tests.sh requires `cast` (for activate(bytes32) calls), but the prereqs section only mentioned Rust + lld. New teammates without stock foundry installed would hit the script's `cast not found` die without anywhere in the doc telling them how to install it.
The EVM impl needs `initialized` as a storage-resident demarcation of the bootstrap window; the Rust impl demarcates that phase differently and doesn't need a slot for the flag at all. Pinning `initialized` at the end of the layout (slot 14, in its own slot) lets the Rust side omit it without disturbing the offset of any other field. With `initialized` no longer sharing slot 8, `adminCount` widens from uint248 to uint256 (its natural counter type — the uint248 only existed to leave the high byte free for the packed bool). The packing codecs (`adminCountFromPacked`, `initializedFromPacked`, `packAdminCountAndInitialized`) are removed along with the `INITIALIZED_BYTE_OFFSET` constant and the factory's `_writePackedBool` helper, all now unused. Tests and the mutation harness are updated to read `adminCountSlot()` and `initializedSlot()` as two independent `vm.load`s. Added a regression assertion that the two slots are disjoint so they can never collapse back together.
`getRoleAdmin`, `setRoleAdmin`, and `_requireRoleAdmin` each carried
a ternary of the form
return adminRole == bytes32(0) && role != DEFAULT_ADMIN_ROLE
? DEFAULT_ADMIN_ROLE : adminRole;
intended to fall through to the default admin for unconfigured roles.
The ternary is a tautology: `DEFAULT_ADMIN_ROLE` is defined as
`bytes32(0)`, which is also the EVM storage zero-default, so both
branches produce the same bytes32 value in every case. The
`role != DEFAULT_ADMIN_ROLE` guard implies a special case that
doesn't exist — DEFAULT_ADMIN_ROLE being its own admin falls out of
the same `bytes32(0)` identity for free.
Collapse all three call sites to raw storage reads (matching OZ
AccessControl's pattern verbatim) and document the load-bearing
identity on the `DEFAULT_ADMIN_ROLE` constant itself so future
readers can find the rationale next to the constant rather than
fragmented across three call sites.
The setRoleAdmin event emission test carried the same dead ternary
to compute the expected `previousAdminRole`; collapse it too.
Behaviorally identical (`bytes32(0) == DEFAULT_ADMIN_ROLE`); all 440
tests still pass.
…ew comments (#63) * rename(factory): ITokenFactory -> IB20Factory, TokenVariant -> B20Variant, createToken -> createB20 Renames the token factory surface to match the B20 naming convention used everywhere else in the codebase (IB20, IB20Stablecoin, IB20Asset, MockB20, ...). - ITokenFactory -> IB20Factory - MockTokenFactory -> MockB20Factory - TokenFactoryTest -> B20FactoryTest - TokenVariant -> B20Variant - createToken() -> createB20() - getTokenAddress() -> getB20Address() - TOKEN_FACTORY_ADDRESS -> B20_FACTORY_ADDRESS - TOKEN_FACTORY -> B20_FACTORY - test/unit/TokenFactory -> test/unit/B20Factory All 394 tests pass unchanged. * fix(factory): address P0 review comments - UnsupportedVersion: add B20Variant as second parameter so callers know which variant rejected their version byte - TokenCreated event: rename to B20Created - ActivationRegistryFeatureList: update B20_FACTORY key from keccak256("base.token_factory") to keccak256("base.b20_factory") (0xceff...8a5b -> 0x7875...b800)
…#62) * chore: reconcile ISO 4217 allowlist with SIX List One (Amendment 180) Brings the fiat allowlist in `test/lib/ISO4217.sol` and the supporting docs back in sync with the canonical SIX-maintained ISO 4217 List One: - Remove ANG (Netherlands Antillean Guilder) — withdrawn 2025-03-31, replaced by XCG under Centrale Bank van Curaçao en Sint Maarten. - Add XCG (Caribbean Guilder) — new multi-jurisdiction X-prefix fiat. - Remove BGN (Bulgarian Lev) — withdrawn 2026-01-01 per Amendment 180 (Bulgaria joined the eurozone at EUR 1 = BGN 1.95583). - Add CLP (Chilean Peso) — previously missing; CLF (the inflation indexing unit) was excluded correctly, but CLP itself had no entry. Allowlist size stays at 155. Docs page gets an Appendix with links to the SIX landing page, List One (XLS + XML), amendments index, and the specific Amendment 180 PDF so future reconciliations have one anchor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: drop string argument from ITokenFactory.InvalidCurrency Narrows the error to a bare `InvalidCurrency()` selector — the offending string was redundant with the calldata the reverting tx already carries, and an argless error is cheaper to emit and assert against. - `ITokenFactory`: change signature + adjust NatSpec on `currency` and `MissingRequiredField` to reference the new shape. - `MockTokenFactory`: revert with `InvalidCurrency()`. - `createToken.t.sol`: assert against `InvalidCurrency.selector` directly (no `abi.encodeWithSelector` wrap needed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "refactor: drop string argument from ITokenFactory.InvalidCurrency" This reverts commit 380731c7055b580679d046aff7c068541d599ca8. * chore: keep ANG and BGN on allowlist as backwards-compat carry-overs Both codes were formally withdrawn from ISO 4217 recently (ANG on 2025-03-31, BGN on 2026-01-01 per Amendment 180), but supply denominated in each is still circulating. Removing them outright would break existing balances; keep accepting both while circulation drains. - Re-add `ANG` and `BGN` to the allowlist with inline comments calling out their withdrawal status and rationale. - Re-add them to the docs supported-currencies table with the withdrawal date noted in the region column. - Update the count from 155 → 157 and add an intro note. - Reframe the Appendix "Recent amendments" subsection as "Recent withdrawals — accepted for backwards compatibility" so the official references stay handy when we revisit the removal decision. `XCG` (the active replacement for ANG) stays on the allowlist; both ANG and XCG are now accepted simultaneously. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: collapse ANG / BGN carry-over comments to trailing one-liners Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* add BURN_FROM_ROLE constant * add missing constant check'
* add interface change only * add necessary mock * move name and version into domain separator, move to b20 * add unit tests
…N_ROLE (#68) Co-authored-by: Cursor <cursoragent@cursor.com>
…stry (#71) * refactor(policy): mirror Rust write_builtins design in MockPolicyRegistry * fix(policy): keep writeBuiltins internal to match precompile ABI
* rename(b20): policyType -> policyName in IB20 token interface and mocks
The bytes32 parameter called policyType on the token side (the slot
identifier passed to policyId() and updatePolicy()) collides semantically
with PolicyType in IPolicyRegistry, which is a BLOCKLIST/ALLOWLIST enum.
They are completely unrelated concepts.
Rename the token-side identifier to policyName, which describes what it
actually is: a named key (e.g. keccak256("TRANSFER_SENDER_POLICY"))
that identifies which authorization gate on the token is being configured.
Affects: IB20.sol, IB20Asset.sol, MockB20.sol, MockB20Asset.sol,
and all B20 policy unit tests. IPolicyRegistry and its PolicyType enum
are unchanged.
* rename: policyName -> policyScope
…144) (#72) * chore: drop ISO 4217 allowlist for a format-only currency check (BOP-144) The stablecoin `currency` field was previously validated against a hand-curated allowlist of ~157 active ISO 4217 fiat codes, with a parallel exclusion list for metals / sentinels / funds codes. That imposed real recurring cost — annual amendments, Solidity / Rust precompile lockstep, per-incident judgement on withdrawn codes that still had supply in circulation (ANG, BGN) — for limited marginal value: the field is self-declared, so consumers using `currency()` for authorization or routing already had to layer their own issuer allowlist on top. This replaces the allowlist with a permissive format check: exactly three uppercase ASCII letters (`A`–`Z`). The check still rejects the typo / shape errors the original problem statement called out (empty string, lowercase, wrong length, symbols like `"USDC"`), and is trivially mirror-able in the Rust precompile with no ongoing sync. Semantic validity ("is this really a real currency?") is now explicitly the consumer's responsibility. - `test/lib/ISO4217.sol`: collapse from ~370 LoC to ~37 LoC. Drop all `bytes3` constants, the dispatch buckets, and the `excludedCount` / `excludedAt` enumerable. Keep the file name and the `isValidFiatCode` symbol so the existing import in `MockB20Factory` and the symbolic `vm.assume(!isValidFiatCode(...))` fuzz filter in `createToken.t.sol` still work unchanged. - `IB20Factory`: NatSpec on `currency`, `MissingRequiredField`, and `InvalidCurrency` updated to describe the format check rather than the allowlist. Error signature unchanged. - Docs: rewrite `currency-validation.md` to lead with the format-only decision and the maintenance-overhead rationale; replace the per-code supported-currencies table with a Risks / Alternatives framing that captures the trade-off. - `createToken.t.sol`: stale "allowlist" comment updated to describe the new format check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: inline the currency format check; delete ISO4217.sol and docs page Follows up the format-only switch by removing the now-vestigial `test/lib/ISO4217.sol` library and the standalone `docs/b20/stablecoin/currency-validation.md` page. The check is small enough that a shared library doesn't earn its weight. - Delete `test/lib/ISO4217.sol` and `docs/b20/stablecoin/currency-validation.md`. - Inline the length-3 + A-Z check directly in `MockB20Factory` where the `currency` field is decoded. - In `createToken.t.sol`, replace the import-based `vm.assume` with a local `_isValidFiatCode` helper so the fuzz filter still works. - Shrink the `currency` param NatSpec in `IB20Factory` to three lines (was ~12). Trim `InvalidCurrency` and `MissingRequiredField` to the essentials; drop the deleted-doc-page references. - Update `IB20Stablecoin.currency()` NatSpec similarly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: drop length constraint from currency format check (A–Z only) Length was a lie in the NatSpec — the only meaningful constraint is the character set. Remove the `length == 3` enforcement from the inline check in `MockB20Factory` and the fuzz-filter helper in `createToken.t.sol`, and reword the `currency` / `InvalidCurrency` / `currency()` NatSpec to reflect "uppercase ASCII letters (A–Z)" with no length claim. Empty strings now pass vacuously, which matches the self-declared framing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: add BatchSizeTooLarge error to IPolicyRegistry Document the registry membership batch limit revert so the Solidity interface matches the Rust PolicyRegistry precompile. Co-authored-by: Cursor <cursoragent@cursor.com> * test: enforce and cover BatchSizeTooLarge in MockPolicyRegistry Per review feedback: with the error declared on the interface, the mock should mirror the Rust precompile's behavior so the same interface-level tests pass in fork-test mode against the real PolicyRegistry. base/base#2876 sets the limit at 64 accounts per call. - MockPolicyRegistry: add `MAX_BATCH_SIZE = 64` (internal) and a pre-mutation length check in `_batchSetMembers` that reverts with `BatchSizeTooLarge(MAX_BATCH_SIZE)`. The check sits at the shared funnel so all three entry points (`createPolicyWithAccounts`, `updateAllowlist`, `updateBlocklist`) inherit it without duplication. - PolicyRegistryTest: expose `MAX_BATCH_SIZE` as a compile-time constant on the test base (kept as a literal, not pulled from the mock, so fork tests against the real precompile share it) and add `_makeAccounts(n)` for batch-bound tests that need to bypass the existing 0..5 `_boundAccounts` cap. - Tests: per affected entrypoint, add one revert test (fuzz over a size above the limit) and one inclusive-boundary success test (batch size exactly == MAX_BATCH_SIZE). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Conner Swenberg <conner.swenberg@coinbase.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ut pins (#75) * test(storage): tighten packed-slot layout pins for Rust cross-validation * refactor(storage): migrate packed policy slots to Solidity packed structs
* add helpers, wire to factory mock * branch cleanup * natspec cleanup * change viz for role helper * use variant specific create param versions
* Add CI workflow: build, test, fmt, coverage Runs on every push to main and every PR: - forge build --sizes (compilation + contract size report) - forge test -v (full test suite with fuzz) - forge fmt --check (formatting gate) - forge coverage --report lcov + Codecov upload (informational, not a gate) Codecov upload requires CODECOV_TOKEN secret to be set in repo settings. * Fix CI: separate compilation from size check forge build --sizes exits non-zero when any contract exceeds EIP-170. MockTokenFactory is a test mock that intentionally exceeds the limit. Separate compilation (forge build) from the informational size report so test mocks do not block CI. * Run forge fmt against rebased main * Pin foundry-toolchain to stable; fix B20FactoryLibTest formatting Nightly foundry has different fmt rules than stable. Pinning to stable aligns CI format checks with local development. Also fixes a 403 rate-limit issue where foundryup would fail fetching release tags from the GitHub API. * Fix formatting for forge v1.7.1 stable The CI stable toolchain is v1.7.1 which removes trailing blank lines before closing braces in empty contract bodies. Update local format to match. * Remove coverage job Not actively monitored and Codecov token is not set up. * Preserve single-line slot helpers in MockB20AssetStorage Use forgefmt: disable-next-item on each function to prevent forge fmt from expanding the compact lookup-table form. * Use forgefmt disable-start/end for MockB20AssetStorage slot helpers Switch from per-function disable-next-item to a single block, and add the same to minimumRedeemableSlot / redeemPolicyIdsSlot.
Signed-off-by: StepSecurity Bot <bot@stepsecurity.io>
eric-ships
force-pushed
the
chore/GHA-280215-stepsecurity-remediation
branch
from
June 2, 2026 22:00
5034726 to
16757d0
Compare
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
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
This pull request has been generated by StepSecurity as part of your enterprise subscription to ensure compliance with recommended security best practices. Please review and merge the pull request to apply these security enhancements.
Security Fixes
Harden Runner
Harden-Runner is an open-source security agent for the GitHub-hosted runner to prevent software supply chain attacks. It prevents exfiltration of credentials, detects tampering of source code during build, and enables running jobs without sudo access.
Pinned Dependencies
Pinning GitHub Actions to specific versions or commit SHAs ensures that your workflows remain consistent and secure.
Unpinned actions can lead to unexpected changes or vulnerabilities caused by upstream updates.
Feedback
For bug reports, feature requests, and general feedback; please create an issue in step-security/secure-repo or contact us via our website.