Skip to content

code simplification pass - #189

Open
kethcode wants to merge 13 commits into
audit/v2.5from
fix/simplification
Open

kethcode wants to merge 13 commits into
audit/v2.5from
fix/simplification

Conversation

@kethcode

@kethcode kethcode commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Use typed calls for strict wrapper and identity-registry reads, reuse the
registry pagination helper, and share the lens aggregation path across
factory counts. Inline single-use policy closure forwarders while keeping
the virtual extension points and their rationale.

Preserve bounded non-reverting wrapper limits, single-factory duplicate
semantics, public interfaces, storage layout, and the unchanged test suite.
Net src reduction: 196 nonblank/noncomment lines, 282 physical lines.

Validation: 907 passing results across 85 suites with the fixed timestamp
and seed, 1,000 fuzz runs, and all three 2,000-run/60,000-call invariant
campaigns. All 111 audited ABIs and normalized storage layouts match the
529f2db baseline; all runtimes fit EIP-170. Both market bytecodes are
unchanged. Typed calls add 2,345 wrapper runtime bytes and 2,768 wrapper
factory bytes; their runtimes remain 11,032 and 14,984 bytes.
Consolidate deposit/mint receipt checks and withdraw/redeem settlement while
preserving validation order, rounding, exact scaled-balance deltas and events.
Share bounded one-word probes for non-reverting limit views, retaining short
return, malformed boolean/address and oversized returndata behavior.

Removes 38 nonblank noncomment source lines (64 physical) relative to 17feb1b.
Wrapper runtime falls 11032 -> 10639 bytes; wrapper factory 14984 -> 14605.
The typed strict reads from the preceding commit remain intact.

Validation: unchanged 907-test fixed timestamp/seed suite, all three full
invariant campaigns, exact ABI and normalized persistent storage layout
matches for all 111 audit artifacts, all runtime sizes below EIP-170, scoped
Forge formatting, Solhint and diff checks. Both market bytecodes are unchanged.
Share raw word and low-bit boolean reads in BaseHooks. Keep checked integer
narrowing at each original call site, including the periodic policy's uint96
minimum-deposit bound. Preserve optional/trailing calldata behavior and all
virtual policy extension points.

Removes 34 nonblank noncomment source lines (49 physical). Runtime changes:
OpenTermHooks +2 bytes, FixedTermHooks +4, PeriodicTermHooks -13.

Validation: unchanged 907-test fixed timestamp/seed suite, all three full
invariant campaigns, exact ABI and normalized persistent storage layout
matches for all 111 audit artifacts, all runtime sizes below EIP-170, scoped
Forge formatting, Solhint and diff checks. Both market bytecodes are unchanged.
Every market-to-hook callback has the same shape: selector, static head
words, the MarketState block, then the caller's trailing calldata as
`extraData`. Replace the ten hand-written encoders and their 63 offset
constants with one builder; each callback writes its head words at the free
memory pointer and calls it. onSetAnnualInterestAndReserveRatioBips keeps
its own two-word return check and uint16 masking. The onExecuteWithdrawal
encoder stays, on the builder, for the existing direct encoder tests;
markets still reject that flag.

Removes 195 nonblank noncomment src lines (208 physical). Runtime:
WildcatMarket 23,985 -> 23,636; WildcatMarketRevolving 24,541 -> 24,192.

Validated on 17feb1b plus this change: 907 passed, 0 failed, 0 skipped
across 85 suites with the fixed timestamp and seed, including the three
2,000-run invariant campaigns; all 111 audited ABIs and storage layouts
unchanged; every runtime below EIP-170.
Replace the 45 single-purpose revert_X assembly helpers in MarketErrors with
selector constants raised through the existing revertWithSelector and
revertWithSelectorAndArgument helpers in Errors.sol. Seven errors whose
helpers hard-coded their selector get constants. Every constant matches
`cast sig` and the selector its helper used, so revert payloads are
byte-identical, including BorrowerTransferWhileSanctioned's address
argument. The MarketEvents.t.sol harness moves to the same helpers.

Removes 263 nonblank noncomment src lines (517 physical). Runtime:
WildcatMarket 23,636 -> 23,506; WildcatMarketRevolving 24,192 -> 24,059.

Validated on 17feb1b plus the previous commit and this change: 907 passed,
0 failed, 0 skipped across 85 suites with the fixed timestamp and seed,
including the three invariant campaigns; all 111 audited ABIs and storage
layouts unchanged; every runtime below EIP-170.
Delete WildcatMarketBase._processExpiredWithdrawalBatch. Nothing in src,
test or script calls it; the lifecycle transition path replaced it. The only
KB mentions are the cross-repo trace table and an older audit excerpt, and
neither ties it to v2.6 work.

closeMarket now uses MarketLifecycleLib.closeFundedState for its four
closed-state writes. The extra isDelinquent = false is recomputed by
_writeState before anything stores or emits it. It also drops the guard
around the pending batch payment: a fully paid batch already returns (0, 0)
from _applyWithdrawalBatchPayment without writes or events.

Manual and automatic closure keep separate batch funding on purpose. Manual
close pays the pending batch first and walks every unpaid batch even at zero
liquidity, which retires sub-wei dust batches; automatic closure gives older
batches priority and stops at zero liquidity.

Removes 23 nonblank noncomment src lines (34 physical). Runtime:
WildcatMarket 23,506 -> 23,444; WildcatMarketRevolving 24,059 -> 23,997.

Validated on 17feb1b plus the previous two commits and this change: 907
passed, 0 failed, 0 skipped across 85 suites with the fixed timestamp and
seed, including the three invariant campaigns; all 111 audited ABIs and
storage layouts unchanged; every runtime below EIP-170.
HooksFactory and HooksFactoryRevolving were near-copies. Move the shared
state, template and instance management, fee pushes, queries and the market
deployment path into an abstract HooksFactoryBase. Each concrete factory
keeps only its constructor, name() and deployment entry points, in their
original statement order. The revolving factory also keeps its versioned
marketData decoding, the commitment-fee getter, and three market-data hooks
that store, clear and emit the fee at their original points in the
deployment.

The interfaces split the same way. IHooksFactoryBase declares the shared
functions; IHooksFactory and IHooksFactoryRevolving keep only their own
deployment functions, plus the revolving errors, event and getter. Both
interface ABIs are unchanged.

Also:
- Encode MarketParameters directly as the constructor payload, replacing
  the two Tmp mirror structs. The payload now also carries the sentinel,
  wrapper factory and identity registry immutables, the constructor-set
  archController, and the resolved borrower principal, which replaces the
  separate principal slot. getMarketParameters returns the same values and
  still reads sphereXEngine when the market asks.
- One _slice helper replaces four identical pagination loops.
- name() returns a plain string in both factories.
- Drop two redundant template existence checks. _deployHooksInstance raises
  the same HooksTemplateNotFound first, and templates are never removed.

Compatibility: storage layouts are unchanged, and so is the standard
factory's ABI. The revolving factory's ABI differs only in entry order and
in addHooksTemplate's second input, now `name` (was `name_`); the selector
is unchanged. Solidity callers that reference shared functions statically
(IHooksFactory.fn.selector, abi.encodeCall(IHooksFactory.fn, ...)) must
name IHooksFactoryBase, and type(IHooksFactory).interfaceId changes; the
factories don't implement ERC-165. HooksTemplateData and one integration
test are updated to match. Both factories now use one transient slot label
for the constructor payload, which is invisible outside the factory since
transient storage is per contract and per transaction.

Removes 755 nonblank noncomment src lines (1,152 physical). Runtime:
HooksFactory 16,568 -> 15,533; HooksFactoryRevolving 17,118 -> 16,061.

Validated on 7af2aed plus this change: 907 passed, 0 failed, 0 skipped
across 85 suites with the fixed timestamp and seed, including the three
invariant campaigns; all 111 audited storage layouts unchanged; every
runtime below EIP-170.
Use one swap-removal path for pull and push role providers, preserving the
packed indices, registry repair, and Removed/Updated event ordering. Inline
the single-use transfer-recipient default check while retaining its exact
short-circuit order and the virtual feature restriction hook.

No public ABI, storage layout, virtual extension point, or test changes.
Removes 32 nonblank noncomment source lines and 39 physical lines.

Validated on 7af2aed plus these two files: 907 passing results across 85
suites with the fixed timestamp and seed, including all three invariant
campaigns at 2,000 runs and 60,000 calls each. All 111 audited ABIs and
normalized storage layouts match the audit baseline; all runtimes fit
EIP-170. BaseAccessControls runtime decreases from 7,255 to 7,206 bytes.
name() and symbol() repeated the same assembly that returns an ABI string
from two packed immutable words. Move it into _returnPackedString. The
returned bytes and both external signatures are unchanged.

Removes 7 nonblank noncomment src lines (11 physical). Runtime:
WildcatMarket 23,444 -> 23,435; WildcatMarketRevolving 23,997 -> 23,988.

Validated on cbd83a3 plus this change: 907 passed, 0 failed, 0 skipped
across 85 suites with the fixed timestamp and seed, including the three
invariant campaigns; all 111 audited ABIs and storage layouts unchanged
(the revolving factory's documented input rename aside); every runtime
below EIP-170.
FeeMath.updateScaleFactorAndFees and updateDelinquency had no callers in
src. Markets accrue through WildcatMarketBase._updateScaleFactorAndFees,
which also applies the post-repayment-date full penalty, yet
docs/protocol/accounting.md pointed readers at the FeeMath copy. Move the
two functions into test/libraries/wrappers/FeeMathExternal.sol as a
reference model built on the live FeeMath primitives, so every existing
FeeMath.t.sol assertion still runs unchanged. FeeMath's own
calculateLinearInterestFromBips duplicated MathUtils' byte for byte; the
model and ProtocolFeeRoundingReview now use the MathUtils version. The
accounting doc links the live accrual path.

KB check: no v2.5 or v2.6 workstream doc or retained feature branch uses
either function. The only KB mentions are a historical cleanup note and an
older audit proof of concept.

Removes 44 nonblank noncomment src lines (84 physical). No runtime change.

Validated on 828ece1 plus this change: 907 passed, 0 failed, 0 skipped
across 85 suites with the fixed timestamp and seed, including the three
invariant campaigns; all 111 audited storage layouts unchanged; every
runtime below EIP-170.
Delete code nothing references:
- FunctionTypeCasts.asReturnsPointers, an adapter for a signature no caller
  uses any more;
- HooksConfigDataLib.kindForVersion, which only forwarded to the
  _kindForVersionHash the lens still uses;
- MarketControllerParameters and ProtocolFeeConfiguration, structs from the
  v1 controller deployment that no contract, ABI or script uses;
- the NotApprovedLender selector constant.

onlyBorrower now raises NotApprovedBorrower through revertWithSelector and
the existing constant instead of repeating the selector in assembly. The
revert payload is identical. The modifier is inlined at every borrower-only
function, so this costs 59 runtime bytes in each market.

KB check: no v2.5 or v2.6 workstream doc or retained feature branch
references any of these. The covenant branches define their own
kindForVersion.

Removes 44 nonblank noncomment src lines (80 physical). Runtime:
WildcatMarket 23,435 -> 23,494; WildcatMarketRevolving 23,988 -> 24,047.

Validated on 828ece1 plus the previous commit and this change: 907 passed,
0 failed, 0 skipped across 85 suites with the fixed timestamp and seed,
including the three invariant campaigns; all 111 audited storage layouts
unchanged; every runtime below EIP-170.
Route repeated market getters through five private typed helpers, retaining
Solidity ABI decoding and the existing read order. This removes duplicated
compiled readers while keeping the previous settlement consolidation.

Against the prior debloat result, wrapper runtime falls by 2,027 bytes and
measured deployment gas by 435,559, for 15 additional source code lines.
Deployment gas is now 0.49% below the audit baseline; average deposit, mint,
withdraw and redeem gas remains 2.5-3.1% above that baseline.

Validation: full fixed-seed suite passes 907 tests in 85 suites, with three
60,000-call invariant campaigns and zero reverts. All 111 artifact ABI and
storage-layout gates pass; only wrapper and wrapper-factory bytecodes change.
Sign up for free to 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.

1 participant