feat: add margin trading periphery#563
Conversation
Scaffolding for the margin-trading periphery suite (M0 of the implementation plan). - morpho-blue @ v1.0.0 (lib/morpho-blue): the lending integration. The adapter will reuse Morpho's own libraries (MarketParamsLib, MorphoBalancesLib, SharesMathLib, MathLib) rather than reimplementing share/accrual math. Only the interfaces (>=0.5.0) and libraries (^0.8.0) are imported — never Morpho.sol (0.8.19) — so everything compiles under this repo's 0.8.26. - solady @ v0.1.26 (lib/solady): LibClone for clones-with-immutable-args. The bundled OpenZeppelin is 5.0.2 whose Clones lacks immutable-args support (added in 5.1) and is shared with v4-core/permit2, so bumping it is avoided. - remappings.txt: add morpho-blue/ and solady/ entries. Verified: imported morpho-blue pragmas admit 0.8.26; morpho-blue's only nested dependency is forge-std (no transitive OZ clash); a throwaway smoke test importing MarketParamsLib/IMorpho/LibClone compiled and passed under 0.8.26.
…Registry)
M1.5 of the margin-trading plan — the custom value types that make illegal
states unrepresentable, built bottom-up and tested in isolation with no mocks
(type-driven-design). Consumed by the adapter, account, and router milestones.
src/types/:
- Direction.sol enum {Long, Short} — replaces a bool long/short flag so
flows branch exhaustively.
- LeverageX18.sol WAD leverage (1e18 = 1x); toLeverageX18 reverts below 1x,
so sub-1x leverage is unconstructable.
- Ltv.sol WAD LTV as a type distinct from amounts and leverage, so
health math cannot unit-confuse them.
- Market.sol the (collateral, debt) pair as a first-class type, plus
toSwapParams — the single choke point that reconciles a v4
pool's currencies with the market and derives zeroForOne
(folds invariant I-11 into one unavoidable runtime check).
- Owner.sol minimal ownership concern (read/write/onlyOwner) composed
by the adapter for governance gating.
- MarketRegistry.sol governed (collateral, debt) -> Morpho MarketParams routing
table; resolve() reverts MarketNotSupported when unset (no
silent default market).
toSwapParams keys on the explicit swap-input currency (open sells debt, close
sells collateral) rather than Direction: zeroForOne is fully determined by the
input currency and canonical pool ordering, and the input is what the flows
actually differ on. Direction lives in the entry-point params. Design doc note
to follow.
M1 of the margin-trading plan. The lending-protocol-agnostic boundary the router and account depend on, consuming the Market type. - src/interfaces/ILendingAdapter.sol: one singleton adapter per lending protocol, parameterized by the Market (collateral, debt) pair. Each encode function returns the call the MarginAccount performs as itself, so no delegated authorization is needed; onBehalf and receiver are owned and re-validated by the account, not trusted from adapter bytes. LTV reads are typed as Ltv. - test/mocks/MockLendingAdapter.sol: minimal configurable implementation so later milestones can compile and test against the interface. Interface and mock compile under 0.8.26.
M2 of the margin-trading plan. The per-user position container, deployed as a Solady clone-with-immutable-args whose owner and manager are baked into the clone bytecode (read via LibClone.argsOnClone), so there is no initializer and ownership is soulbound. The account is the borrower and supplier in the lending protocol, so it acts as itself and needs no delegated authorization. It owns the authority-bearing fields rather than trusting adapter-encoded bytes: - every privileged primitive is gated to the manager or owner (NotAuthorized); - onBehalf is always address(this); - every fund recipient is constrained to the manager or owner (ReceiverNotAllowed); - each adapter-encoded call is asserted to target the adapter's lending protocol and carry no value, then run as a regular call, never a delegatecall; - execute is an owner-only escape hatch so the owner can always close or recover without the manager. Repay returns the assets actually repaid as the account's debt-token balance decrease, which makes repay-all correct without reimplementing share math. Tests cover the access gate (including a fuzzed unauthorized caller), the recipient constraint, the onBehalf invariant, the target check, owner-only execute, repay-all, sweep, and a no-delegatecall proof (the call target's storage changes while the account's does not). 15 tests pass. Adds a MockLendingProtocol target and a forced-target option on MockLendingAdapter.
M3 of the margin-trading plan. Deploys per-user MarginAccount clones via Solady clone-with-immutable-args, baking (owner, manager) into the clone bytecode. - accountOf(owner, subId) predicts the CREATE2 address for any owner without deploying. - createAccount(owner, subId) is idempotent: it returns the existing account if already deployed, tolerating a lost lazy-deploy race. - The salt binds owner, manager, and subId. Binding the manager keeps addresses distinct across router versions and keeps accountOf collision-free; binding the owner neutralizes address squatting, since deploying at someone's predicted address bakes them in as the owner. 8 tests pass, including squatting neutralization, idempotent deploy, and determinism across owners, subIds, and managers.
M4 of the margin-trading plan. The first ILendingAdapter: a singleton over all curated Morpho Blue markets, modeled as a thin shell composing a governed (collateral, debt) routing table and an owner guard. All encode and read logic reuses morpho-blue's own libraries, so no Morpho math is reimplemented. - setMarket is owner-gated and requires the market to already exist on Morpho (idToMarketParams check), then registers the full MarketParams keyed by its token pair. Encode and read calls resolve the pair and revert MarketNotSupported for unrouted pairs. - Encodes map to Morpho calls with the account as onBehalf and empty data (so no Morpho callback fires); full repay burns the account's borrow shares via Morpho's shares path. lendingProtocol returns the Morpho singleton. - positionOf returns raw collateral plus interest-accrued debt via MorphoBalancesLib.expectedBorrowAssets; currentLtvWad combines accrued debt with the market oracle price (1e36 scale); maxLtvWad returns the market lltv. Unit tests (9) cover gating, the not-created and not-supported reverts, encode target/onBehalf/empty-data, and shares-based full repay, using a minimal MockMorpho. The accrual-dependent reads (positionOf debt, currentLtvWad) read live market state and will be validated by a mainnet fork test, which is pending a fork RPC and verified Morpho market addresses.
M5 of the margin-trading plan. Defines the margin action opcode space and the calldata decoders the router uses to dispatch margin actions. - MarginActions: opcodes 0x1c through 0x21 (supply, withdraw, borrow, repay, sweep, assert-health), extending the Actions space that ends at 0x1b. Opcodes below 0x1c fall through to the inherited V4Router handlers. There is no market/swap reconciliation opcode: that check lives in the single Market.toSwapParams choke point at plan-build time, so it cannot be skipped. - MarginCalldataDecoder: abi-decode based decoders for the action param blobs. abi decoding rather than hand-rolled calldata slicing is deliberate, since these params are not the hottest path and decode safety matters here. 6 tests: decoder round-trips (including a fuzz round-trip) and opcode contiguity and disjointness from the inherited Actions space.
M6 of the margin-trading plan. The router composes the v4 action machinery (V4Router, ReentrancyLock, Permit2Forwarder, Multicall_v4, NativeWrapper), implements _pay (Permit2 two-payer), msgSender (returns the locker, which is load-bearing for deriving the caller's account), and overrides _handleAction to dispatch the margin opcodes while falling through to the inherited swap, take, and settle handlers. Each leveraged position is built as a single flash-style swap inside one PoolManager unlock: borrow the debt, swap it into collateral, supply the collateral, then draw the debt to settle. The active account is held in transient storage, always derived from the authenticated caller via the factory, never from calldata. The router deploys and is the manager of its factory, so it can drive each account's lending primitives. - openPosition: pulls equity via Permit2, then assembles swap-out / take / supply / borrow / settle. - closePosition: buys the current debt, repays it, withdraws collateral, settles the swap, and returns residual collateral (realized PnL) to the caller. - addCollateral: pulls collateral and supplies, no swap or unlock needed. - The single Market.toSwapParams choke point validates the pool against the market and derives swap direction for every flow. V4Router._handleAction is made virtual (backward compatible, no behavior change) so the action set can be extended; all existing V4Router and PositionManager tests still pass. Unit tests (5) cover the factory/manager wiring, accountOf passthrough, and the pre-unlock guards (slippage-bound-required, deadline). The swap-coupled leverage flows run through a real PoolManager and are validated by the integration and fork suite (next), which also needs a fork RPC and verified Morpho markets.
M7 (part 1). End-to-end tests of the router's leverage flows against a real local PoolManager and a deep 1:1 pool, with a mock lending protocol standing in for Morpho. These validate that the flash-style plan assembly nets to zero. - open: equity plus a borrowed exact-output buy produces a collateral position of equity + bought, draws debt within the slippage bound, and leaves nothing loose in the account or router. - close: buys and repays the current debt, withdraws all collateral, settles the swap, and returns residual collateral (realized PnL) to the caller; debt and collateral both end at zero. MockLendingAdapter.positionOf now reflects the live mock-protocol state so the close and withdraw paths read real debt and collateral.
M7 (part 2). Forks mainnet and exercises MorphoLendingAdapter against the real Morpho Blue WETH/USDC market, validating the accrual-dependent reads the unit tests could not (positionOf accrued debt, currentLtvWad from the live oracle). The market and token addresses are verified on-chain in setUp (idToMarketParams must return the expected tokens) rather than trusted blindly. The test supplies 1 WETH, borrows 1000 USDC under the 0.86 LTV cap, checks the accrued position and LTV, and repays in full via the shares path. Skips when MAINNET_RPC_URL is unset.
Completes the user-facing position API. - increasePosition adds leverage to an existing position, sharing the lever-up plan with openPosition (open lazily creates the account; increase operates on the existing one). - decreasePosition partially delevers: it sells collateral to buy and repay a chosen amount of debt, leaving the position open and smaller, and asserts the resulting LTV against maxLtvAfter (zero skips the check). - The withdraw handler's OPEN_DELTA now resolves to the collateral the swap owes the pool (used by partial delever), symmetric with the borrow handler; close withdraws the explicit full collateral it reads before the unlock. - ASSERT_HEALTH is wired into decrease and treats a zero bound as skip. Integration tests cover increase (collateral and debt grow), decrease (both shrink, position stays open), and the resulting-LTV-too-high revert.
Validates that the router pulls the caller's equity into their account through a real Permit2 deployment (rather than the equity being pre-funded), then builds the leveraged position. Confirms the permit2.transferFrom path used by open, increase, and add-collateral.
Security review (High): closePosition repaid a stale, rounded-up asset amount of debt and then withdrew ALL collateral. Against real Morpho an asset-denominated repay converts to shares rounding down, leaving dust borrow shares; withdrawing all collateral with non-zero shares then fails Morpho's health check (INSUFFICIENT_COLLATERAL), so the close path reverted for any interest-bearing position. The local mock hid this (no accrual, asset-based), and close was never fork-tested. Fix: repay all by shares (type(uint256).max) so the borrow shares reach zero before the full-collateral withdrawal. The swap still buys exactly the read debt (expectedBorrowAssets), which equals what a full-share repay pulls in the same block, so the account holds exactly enough and nothing is left over. The fork test now warps a day to accrue interest, repays by shares, and withdraws all collateral, asserting both the debt and collateral reach zero and the collateral is returned. This reproduces and guards the close sequence against real Morpho.
…ral guard Addresses the security review's medium findings. - Adapter allowlist (Medium): the router now only accepts governance-allowlisted lending adapters on every flow (AdapterNotAllowed otherwise), enforcing the documented trust model. A hostile caller-supplied adapter could otherwise siphon the caller's own equity. Governance defaults to the deployer and can be handed off via transferGovernance; setAdapterAllowed is governance-gated. - Mandatory delever health (Medium): decreasePosition now requires a non-zero maxLtvAfter, so a delever cannot silently skip the resulting-LTV check and leave the position less healthy than intended. - addCollateral now reverts on a zero amount instead of deploying an account and reverting later. Tests: a non-allowlisted adapter reverts AdapterNotAllowed, only governance can set the allowlist, and governance defaults to the deployer; the integration suites allowlist their adapter in setUp.
… markets Addresses the security review's low findings. - MarginAccountFactory reverts ZeroAddress if constructed with a zero implementation or manager. - MorphoLendingAdapter.MarketSet now emits the oracle, irm, and lltv so offchain monitoring can vet the routed market's parameters, not just its token pair. - Documents that supported markets are standard ERC20s only (the adapter allowlist curates Morpho markets, which exclude fee-on-transfer and rebasing tokens), under which every router flow nets to zero with no residual. This is the resolution for the residual-sweep finding.
Adds PositionOpened, PositionIncreased, PositionClosed, PositionDecreased, and CollateralAdded events, each carrying the owner, account, market currencies, and the relevant amount, so offchain indexers can reconstruct the position lifecycle from the router rather than only from the factory, PoolManager, and Morpho. Validated with an expectEmit on the open flow.
openPosition, increasePosition, and addCollateral are now payable. When native ETH is sent, the router wraps it to WETH and credits the account as equity (the market collateral must be WETH, else NativeCollateralMismatch); otherwise the ERC20 Permit2 pull is used. This lets users lever the ETH they hold without wrapping it themselves first. Test covers addCollateral with native ETH (wrapped and supplied as WETH, no ETH left in the router) and the non-WETH-collateral revert. The wrap path is shared by the open and increase flows.
Instruments the margin suite with vm.snapshotGasLastCall, matching the repo's existing gas-snapshot convention, and commits the isolate-mode baseline: - MarginRouter open / close / increase / decrease / add-collateral (native) - MarginAccount borrow / repay - MarginAccountFactory createAccount (clone deploy) forge snapshot --check (and forge test --isolate) now track gas regressions for these contracts. No existing snapshots changed.
Reorders members to a single convention: state (using, constants, immutables, storage) -> errors -> events -> modifiers -> constructor -> open entrypoints -> privileged/admin entrypoints -> internal/private, with intuitive ordering inside each group. No behavior change (full suite and gas snapshots unchanged). - MarginRouter: modifier moved before the constructor; the user flows (open/increase/close/decrease/addCollateral) and public views grouped as open entrypoints; setAdapterAllowed/transferGovernance grouped as admin; the framework overrides and helpers moved to the internal/private section. - MorphoLendingAdapter: the ILendingAdapter view surface and owner() grouped as open entrypoints; setMarket/transferOwnership moved after them as admin. - MarginAccountFactory: error declared before the event. MarginAccount already followed the convention.
Documents every margin contract, interface, type, and library to the repo's NatSpec standard. - Contract/interface/library/type-file level: @title, @author, @notice, and @Custom:security-contact on the four deployable contracts. - Every external/public function has @notice plus @param and @return (with units: WAD, token decimals, X36, bps); implementations use @inheritdoc with @dev for implementation-specific notes, keeping the canonical docs on the interfaces. - Internal and free functions documented; every struct field, error, and event parameter documented; the modifier and the MarginActions opcodes documented. Documentation only, no logic, signature, or ordering changes. forge build passes, the full suite (754 tests) is green, and gas snapshots are unchanged.
Convert MarginAccountFactory from a separately-deployed contract into an abstract mixin inherited by MarginRouter. The factory's clone-deployment and deterministic-addressing logic stays in its own file for cleanliness, but the router is now the manager baked into every account directly, removing the standalone factory deployment and an external-call hop on every open, increase, and add-collateral flow. - MarginAccountFactory: abstract, single accountImplementation_ ctor arg, manager fixed to address(this), accountOf/createAccount public virtual - MarginRouter: inherits the mixin, drops the factory immutable and the new MarginAccountFactory deployment, calls accountOf/createAccount directly, overrides accountOf for IMarginRouter + mixin - Tests updated to a concrete FactoryHarness; router asserts manager() and determinism in place of the removed factory() getter Gas: open/increase/decrease/close and native add-collateral all drop ~2.5-3.2k gas from removing the external factory call.
Prove the entire margin stack composes, not just each component in
isolation. On a mainnet fork the test drives one continuous lifecycle
through the real contracts:
MarginRouter unlock + flash accounting + delta resolution
-> V4Router swap through a real PoolManager
-> MorphoLendingAdapter encodes real Morpho Blue calls
-> MarginAccount executes them as itself on the live WETH/USDC market
-> real AdaptiveCurveIRM interest accrual
The lending leg, equity tokens (WETH/USDC), Permit2, and WETH9 are the
live mainnet contracts. The only locally deployed venue is the v4 pool,
seeded with deep liquidity at the live Morpho oracle price so the swap
leg and the lending leg agree on valuation; the PoolManager code itself
is the real v4-core contract.
Lifecycle exercised in sequence: open (equity via real Permit2) ->
addCollateral (native ETH, wrapped) -> increase (pure leverage) -> warp
one day (interest accrues from the real IRM) -> decrease (partial
delever, health-bounded) -> close (full unwind, residual PnL returned).
Each stage asserts the real position state through the adapter and that
no dust is left in the account or router.
Gated on MAINNET_RPC_URL; skips cleanly when unset, matching the
existing adapter fork test.
The single-borrower lifecycle is an unrealistically clean path: nobody else touches the market or the pool. Add a scenario with three independent positions contending for the same Morpho market and the same v4 pool at once, to prove isolation under concurrency. Two distinct owners, one of whom runs two sub-accounts, open interleaved (Permit2 and native-ETH equity, different sizes/leverage up to ~0.71 LTV). They accrue interest together for a week from the one shared IRM, one borrower levers up while the others are open, and they are closed in a different order than opened. Asserts the invariants a single borrower cannot exercise: (owner, subId) yields distinct accounts; another borrower's open, increase, or close never moves my collateral and only moves my debt within share-rounding; every owner receives their own residual PnL; the router never retains dust. An external lender supplies USDC up front so the real market can fund several borrowers at once, keeping the market and its accounting real.
The margin action opcodes started at 0x1c, packed immediately against the inherited v4-periphery Actions space (which ends at 0x1b). That left no room for new core actions (swap, settle, take, and similar) to be added without colliding with the margin range. Move the margin opcodes to start at 0x30 (0x30-0x35), reserving the 0x1c-0x2f block for future core actions. The dispatch boundary in _handleAction keys off the lowest margin opcode symbolically, so it tracks the new start automatically; only comments and the explicit opcode-value assertions in the decoder test needed updating. Gas is unchanged: the opcode is a single-byte dispatch key, not packed state.
ActionConstants.OPEN_DELTA is zero, which doubles as both the use-full-delta sentinel and a literal zero amount. Several flows fed a literal zero into the exact-output swap, which the PoolManager rejects with SwapAmountCannotBeZero, producing an opaque revert. - closePosition now resolves the position first and, when the debt is zero (funded only via addCollateral, repaid out of band, or fully liquidated), withdraws the collateral straight to the caller with no swap. The maxCollateralIn slippage bound only gates the swap path. - decreasePosition rejects a zero debtToRepay with SlippageBoundRequired. - _open (open/increase) rejects a zero collateralToBuy with SlippageBoundRequired.
closePosition returned the router's entire collateral balance to the caller, relying on the router holding zero between calls. Because the residual was balance-based rather than delta-based, any stray or donated balance in the collateral token would be swept to whoever closed a position next. Snapshot the router's collateral balance immediately before the unlock and return only the increase across it, so a pre-existing balance is left untouched. The zero-debt swap-free path is unaffected: it withdraws straight to the caller.
closePosition and decreasePosition required the adapter to be on the governance allowlist. If governance removed an adapter while users held positions backed by it, those users could no longer close or delever through the router. Drop the allowlist requirement from the exit flows. The allowlist now gates only exposure-increasing operations (open, increase, add collateral). Unwinding is safe regardless: these flows operate only on the caller's own account, and the MarginAccount itself constrains the call target, receiver, and value.
The margin swaps are all single-hop exact-output, and maxDebtIn (open/increase) and maxCollateralIn (close/decrease) are mandatory non-zero, so they fully bound the worst-case swap input. Strengthen the NatSpec to state that these absolute caps are the binding slippage protection and should be derived from a quote, and that minHopPriceX36 is an optional additional per-hop bound whose zero value disables only that secondary check, not the absolute cap.
| /// @notice The authenticated caller for the current lock. Overrides `BaseActionsRouter.msgSender` | ||
| /// to return the address stored by `ReentrancyLock._getLocker`, which is set to | ||
| /// `msg.sender` at the start of each `isNotLocked` call. The active account is derived | ||
| /// from this value, so correctness here is load-bearing for the entire position system. | ||
| function msgSender() public view override returns (address) { | ||
| return _getLocker(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there something we can borrow from v4 core here? noting the nested locking funtionality
| /// @inheritdoc IMarginRouter | ||
| function openPosition(OpenParams calldata params) | ||
| external | ||
| payable | ||
| isNotLocked | ||
| checkDeadline(params.deadline) | ||
| returns (address account) | ||
| { | ||
| account = _open(params); | ||
| emit PositionOpened(msgSender(), account, params.market.collateral, params.market.debt, params.collateralToBuy); | ||
| } | ||
|
|
||
| /// @inheritdoc IMarginRouter | ||
| function increasePosition(OpenParams calldata params) | ||
| external | ||
| payable | ||
| isNotLocked | ||
| checkDeadline(params.deadline) | ||
| returns (address account) | ||
| { | ||
| account = _open(params); | ||
| emit PositionIncreased( | ||
| msgSender(), account, params.market.collateral, params.market.debt, params.collateralToBuy | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there a specific reason you want to expose both open and increasePosition? Seem to be doing the same thing under the hood (unlke mint/increase from v4)
There was a problem hiding this comment.
fair point, they really are the same thing just with different names. earlier on i was keeping track of positions and increase only worked on existing positions but for most lending protocols there is no difference so they ended up converging. can remove.
There was a problem hiding this comment.
yeah and seeing as you only conditionally deploy a new margin account seems fine to just condense these
| if (params.collateralToBuy == 0) revert SlippageBoundRequired(); | ||
| if (params.maxDebtIn == 0) revert SlippageBoundRequired(); |
| /// @param payer The address bearing the payment. `address(this)` means the router holds the | ||
| /// tokens (e.g. after borrowing debt from the account); any other address is an EOA or | ||
| /// contract paying via Permit2. | ||
| /// @param amount The amount to transfer, in the token's native decimals. | ||
| function _pay(Currency currency, address payer, uint256 amount) internal override { | ||
| if (payer == address(this)) { | ||
| currency.transfer(address(poolManager), amount); | ||
| } else { | ||
| permit2.transferFrom(payer, address(poolManager), uint160(amount), Currency.unwrap(currency)); | ||
| } | ||
| } | ||
|
|
||
| /// @notice Stores the active account for the current unlock in transient storage (EIP-1153). | ||
| /// It is always derived from the authenticated caller stored by `ReentrancyLock`, never | ||
| /// from calldata, so margin handlers operate only on the caller's own account. | ||
| /// @param account The MarginAccount address to store, or `address(0)` to clear after unlock. | ||
| function _setActiveAccount(address account) private { | ||
| bytes32 slot = ACTIVE_ACCOUNT_SLOT; | ||
| assembly ("memory-safe") { | ||
| tstore(slot, account) |
There was a problem hiding this comment.
nit these would be cleaner imo as library functions instead of private
| // single choke point: validate the pool matches the market and derive the swap direction. | ||
| // opening sells the debt to buy the collateral. |
| actionParams[2] = abi.encode(params.market.collateral, account, ActionConstants.OPEN_DELTA); | ||
| // supply the account's full collateral balance (equity + bought) | ||
| actionParams[3] = abi.encode(params.adapter, params.market, uint256(ActionConstants.OPEN_DELTA)); | ||
| // borrow the debt owed for the swap, sent to the router for settling |
There was a problem hiding this comment.
cleaner as a modifier given you have to clear the slot?
| (ILendingAdapter adapter, Market memory market, uint256 amount) = params.decodeAdapterMarketAmount(); | ||
| if (amount == 0) amount = IERC20(Currency.unwrap(market.collateral)).balanceOf(account); | ||
| IMarginAccount(account).supplyCollateral(adapter, market, amount); |
There was a problem hiding this comment.
what is OPEN_DELTA? is that 0? if so use that constant in the equality check here
| } else if (action == MarginActions.ACCOUNT_REPAY) { | ||
| (ILendingAdapter adapter, Market memory market, uint256 amount) = params.decodeAdapterMarketAmount(); | ||
| IMarginAccount(account).repay(adapter, market, amount); | ||
| } else if (action == MarginActions.ACCOUNT_SWEEP) { | ||
| (Currency currency, uint256 amount, address to) = params.decodeSweep(); | ||
| IMarginAccount(account).sweep(currency, amount, to); |
There was a problem hiding this comment.
essentially funds are hanging out in the account after some operations and whether that's where it should live or it should be swept to the caller or some other address is a user preference that is best expressed explicitly imo
| view | ||
| returns (address target, uint256 value, bytes memory callData); |
There was a problem hiding this comment.
Consider tradeoffs of using a Call struct here. Especially given you don't support nonzero value
…ion per subId The Aave v3 and v4 adapters' currentLtvWad/positionOf read account-level state (Aave tracks health across the whole account), so they equal a single position's values only when the (owner, subId) account holds one Aave position on that Pool/Spoke. Correct the NatSpec, which wrongly claimed the router maintains a one-position-per-account invariant: it does not. State the usage requirement plainly (a distinct subId per Aave position) in both adapters and in the margin-trading guide; co-locating two markets on one Aave deployment under one subId blends the reads and can revert or mis-scope a close/decrease. Morpho markets are isolated and unaffected. No functional change.
| /// @param _pending The address proposed as the next owner, or `address(0)` when none is pending. | ||
| /// Read via `pendingOwner()`; set via `propose()`; cleared on `acceptOwnership()`. | ||
| struct Owner { | ||
| address _inner; |
There was a problem hiding this comment.
nit but why not _current? Feels easier to read Owner._current
| address account = _activeAccount(); | ||
| if (action == MarginActions.ACCOUNT_SUPPLY_COLLATERAL) { | ||
| (ILendingAdapter adapter, Market memory market, uint256 amount) = params.decodeAdapterMarketAmount(); | ||
| if (amount == 0) amount = IERC20(Currency.unwrap(market.collateral)).balanceOf(account); |
There was a problem hiding this comment.
theres a currency library so could just write
market.collateral.balanceOf(account) without the unwrapping and casting
| /// @param value The encoded call value; must be zero. | ||
| /// @param callData The calldata to forward. | ||
| /// @return The raw bytes returned by the lending protocol call. | ||
| function _execCall(ILendingAdapter adapter, address target, uint256 value, bytes memory callData) |
There was a problem hiding this comment.
You could just omit target and call adapter.lendingProtocol()
| /// routable. Managed via `setMarket`. | ||
| /// @param owner The current adapter owner, gating `setMarket` and `transferOwnership`. | ||
| struct AdapterStore { | ||
| mapping(Currency collateral => mapping(Currency debt => bool)) allowed; |
There was a problem hiding this comment.
keccak256(collateral, debt) => bool?
| /// @param owner The current adapter owner, gating `setMarket` and `transferOwnership`. | ||
| struct AdapterStore { | ||
| mapping(Currency collateral => mapping(Currency debt => bool)) allowed; | ||
| Owner owner; |
There was a problem hiding this comment.
Could be separated unless there's a specific reason for the AdapterStore struct
|
|
||
| /// @notice Reverts `MarketNotSupported` unless the `(collateral, debt)` pair is allowlisted. | ||
| /// @param market The market pair to check. | ||
| function _require(Market calldata market) internal view { |
There was a problem hiding this comment.
Could be renamed to be more descriptive. Feels weird to have an internal func called require
| /// @dev This is a runtime check (Solidity cannot make set-equality a compile-time guarantee); its | ||
| /// value is locality and unavoidability: every swap passes through this function and it cannot | ||
| /// be bypassed. |
There was a problem hiding this comment.
| /// @dev This is a runtime check (Solidity cannot make set-equality a compile-time guarantee); its | |
| /// value is locality and unavoidability: every swap passes through this function and it cannot | |
| /// be bypassed. |
| uint160 sqrtPriceLimitX96, | ||
| PoolKey memory key | ||
| ) pure returns (SwapParams memory params) { | ||
| // the pool must trade exactly this market's two currencies (order-independent) |
There was a problem hiding this comment.
| // the pool must trade exactly this market's two currencies (order-independent) | |
| // the pool must trade exactly this market's two currencies |
| { | ||
| MarketParams memory marketParams = store.markets.resolve(market); | ||
| bytes memory data; | ||
| return (address(morpho), 0, abi.encodeCall(IMorphoBase.supplyCollateral, (marketParams, amount, account, data))); |
| uint256 collateral = uint256(morpho.position(marketParams.id(), account).collateral); | ||
| uint256 debt = morpho.expectedBorrowAssets(marketParams, account); | ||
| // collateral value expressed in loan-token units | ||
| uint256 collateralValue = collateral * IOracle(marketParams.oracle).price() / ORACLE_PRICE_SCALE; |
There was a problem hiding this comment.
haven't looked at morpho in detail but there is risk of overflow here, .price() says it returns values scaled by 1e36
| /// @param oracle The price oracle address for this Morpho market. | ||
| /// @param irm The interest rate model address for this Morpho market. | ||
| /// @param lltv The liquidation LTV for this Morpho market (WAD, 1e18 == 100%). | ||
| event MarketSet(address indexed collateral, address indexed debt, Id id, address oracle, address irm, uint256 lltv); |
There was a problem hiding this comment.
index id? also worth putting that first?
| function transferOwnership(address newOwner) external { | ||
| store.owner.onlyOwner(msg.sender); | ||
| store.owner.propose(newOwner); | ||
| } |
There was a problem hiding this comment.
this func is repeated in each lending adapter, worth just putting in a base shared contract
| /// @param newOwner The address proposed as the next owner. | ||
| /// @return The same storage reference, for chaining. | ||
| function propose(Owner storage self, address newOwner) returns (Owner storage) { | ||
| if (newOwner == address(0)) revert ZeroOwner(); |
There was a problem hiding this comment.
imo being able to pass in address(0) is a valuable feature ot be able to reverse the proposal of an owner? if you pass in address(0) by accident its not like its a problem
There was a problem hiding this comment.
the top of the contract says
A zero-address successor is
/// rejected, so the role can never be transferred to an unrecoverable address.
but i dont understand how proposing the 0 address would ever actually transfer to an unrecoverable address? the whole point in 2 step proposals is that they have to accept
| /// @param self The `Owner` storage to update. | ||
| /// @param caller The address accepting ownership; typically `msg.sender`. | ||
| function acceptOwnership(Owner storage self, address caller) { | ||
| if (self._pending == address(0) || caller != self._pending) revert NotPendingOwner(caller); |
There was a problem hiding this comment.
| if (self._pending == address(0) || caller != self._pending) revert NotPendingOwner(caller); | |
| if (caller != self._pending) revert NotPendingOwner(caller); |
| { | ||
| (address ownerAddr, address managerAddr) = _authCaller(); | ||
| _requireReceiver(to, ownerAddr, managerAddr); | ||
| IERC20 collateral = IERC20(Currency.unwrap(market.collateral)); |
There was a problem hiding this comment.
i dont think you need to cast to a token, all these functions are available on the currency type
or if everything in this protocol has to be a token (not native), then it feels better to use the ERC20 type by default not currency?
| /// manager of every account it deploys, so it can drive their lending primitives. | ||
| /// | ||
| /// Supported markets are restricted to the governance allowlist of lending adapters, which | ||
| /// curate standard ERC-20 markets only (Morpho Blue does not support fee-on-transfer or |
There was a problem hiding this comment.
"Morpho Blue does not support fee-on-transfer..."
Morpho specific statements should probably land in the Morpho adapter, while in the MarinRouter, it should stay more generalized
| /// @notice Deploys the margin router. | ||
| /// @param poolManager_ The v4 PoolManager singleton the router unlocks for every position flow. | ||
| /// @param permit2_ The Permit2 contract used to pull caller equity and settle swaps. | ||
| /// @param weth9_ The canonical WETH9 contract used to wrap native ETH equity. |
There was a problem hiding this comment.
nit: saying "wrap native token equity" instead of naming ETH directly makes this naming more in line with non mainnet chains
currentLtvWad multiplied collateral by the oracle price (1e36-scaled) before dividing by ORACLE_PRICE_SCALE. For a large collateral position the intermediate product can exceed uint256 and revert, bricking the LTV read. Switch to Math.mulDiv so the product is carried in full 512-bit precision, matching Morpho's own valuation and removing the phantom- overflow path. Result is identical for all non-overflowing inputs.
openPosition and increasePosition took the same OpenParams and ran the same _open path, differing only in the event emitted. Collapse them into a single openPosition: opening into an account that already holds a position simply adds leverage. The factory's AccountCreated event still marks the first open, so the open-vs-increase distinction is preserved for indexers without a redundant entry point. Removes the PositionIncreased event and updates callers, tests, the integration gas snapshot, and the margin-trading doc. The duplicate zero-collateral revert test is dropped; the second-open leverage path is retained under openPosition naming.
The three lending adapters each carried an identical Owner field in their AdapterStore plus the same owner/pendingOwner/acceptOwnership/ transferOwnership functions and onlyOwner guard. Pull that boilerplate into a shared OwnableAdapter base: it holds the Owner guard, exposes the ownership ABI, and offers an _onlyOwner helper the adapters route their setMarket calls through. Each adapter now only declares its market table. The adapters are non-upgradeable singletons, so relocating the guard ahead of each store is layout-safe. Behavior is unchanged (all ownership tests pass); the ShortInverse gas snapshot is regenerated for the smaller adapter bytecode.
Mechanical cleanups from PR review, no behavior change: - Owner: rename the `_inner` field to `_current`; drop the redundant `_pending == address(0)` branch in acceptOwnership (a real caller can never equal a zero pending slot). - MarginRouter: compare against ActionConstants.OPEN_DELTA instead of a bare 0 for the full-balance/full-debt sentinels; read collateral via Currency.balanceOf instead of casting to IERC20; keep the contract doc protocol-agnostic and say "native token" rather than ETH. - Aave/Aave v4 adapters: rename the `_require` helper to `_requireSupportedMarket` / `_resolveRoute`. - Morpho adapter: inline the empty callback bytes, index the market `id` in MarketSet and move it first, and document the no-FoT/rebasing constraint here rather than in the router. Integration and ShortInverse gas snapshots regenerated.
AdapterStore originally grouped the market table with the Owner guard. Now that the guard lives in OwnableAdapter, the struct wraps a single field and only adds `store.` indirection. Replace it in each adapter with the bare mapping (_allowed / _routes / _markets), matching the codebase's _-prefixed internal-state convention. The single-field struct and the bare field occupy the same slot, so storage layout, bytecode, and gas are unchanged (no snapshot movement).
…USDC market The deployment registered a WETH/USDC market whose oracle (0xdC6fd583...) differs from the canonical mainnet market (id 0x94b823e6...), hashing to an unlisted market id with negligible liquidity. The registry keys one market per (collateral, debt) pair, so registering the canonical params evicts the stale entry in one call. The script is idempotent, verifies the routed params byte-for-byte before and after, and optionally warns about residual positions stranded in the stale market.
Reconstructs the full transaction-history field set from public onchain data alone: entry/exit prices from same-tx lending Borrow events (exact fill incl. fees), equity from supplied-minus-bought, venue from which lending protocol fired, pool sub-labels from v4 Initialize metadata, and liquidations from Morpho Liquidate / Aave LiquidationCall attributed to margin accounts. No trading-api write-through required, so third-party integrators can serve identical history. Aave v4 flow indexing pends the Spoke event ABI. Added sample GraphQL queries for the margin indexer: Owner account discovery, portfolio and active/completed tabs, per position history feed, pool sub-label join, raw lending feed including escape hatch operations, registries, and pagination. Field scaling notes for X18 prices and raw token amounts.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
![semgrep-code-uniswap[bot]](https://avatars.githubusercontent.com/u/36115574?s=60&v=4){kind=small}
| } | ||
| } | ||
| }, | ||
| "node_modules/vite": { |
There was a problem hiding this comment.
High severity vulnerability may affect your project—review required:
Line 3314 lists a dependency (vite) with a known High severity vulnerability.
ℹ️ Why this matters
Affected versions of vite and vite-plus are vulnerable to Exposure of Sensitive Information to an Unauthorized Actor / Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal'). Vite's server.fs.deny blocklist—which protects sensitive files such as .env and certificate files from being served—can be bypassed on Windows using alternate path representations (NTFS Alternate Data Stream syntax like /.env::$DATA?raw, or 8.3 short filenames), allowing an attacker to read otherwise-denied files when the dev server is exposed to the network.
References: GHSA
To resolve this comment:
Check if you expose the Vite dev server or vite-plus to the network by configuring a non-loopback address using the --host CLI flag on Windows.
- If you're affected, upgrade this dependency to at least version 6.4.3 at indexer/package-lock.json.
- If you're not affected, comment
/fp we don't use this [condition]
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
| "url": "https://github.com/sponsors/sindresorhus" | ||
| } | ||
| }, | ||
| "node_modules/esbuild": { |
There was a problem hiding this comment.
Medium severity vulnerability may affect your project—review required:
Line 1841 lists a dependency (esbuild) with a known Medium severity vulnerability.
ℹ️ Why this matters
Affected versions of esbuild are vulnerable to Origin Validation Error. esbuild's development server responds to every request, including Server-Sent Events connections, with Access-Control-Allow-Origin: *. Any website a developer visits can therefore make cross-origin requests to the local dev server and read the responses, leaking bundled source code, source maps, and served file paths. Starting the dev server via serve() reaches the vulnerable code path.
References: GHSA
To resolve this comment:
Check if you run esbuild with the --serve flag to start the development server.
- If you're affected, upgrade this dependency to at least version 0.25.0 at indexer/package-lock.json.
- If you're not affected, comment
/fp we don't use this [condition]
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
Add ILendingAdapter.describePosition returning collateral, debt, maxLtv, currentLtv, and health factor in one call, so integrators no longer stitch positionOf + maxLtvWad + currentLtvWad. Aave adapters surface their native health factor (previously discarded); Morpho derives it. Enrich the MarginRouter position events (PositionOpened/Closed/Decreased, CollateralAdded) with amount deltas, resulting totals, LTV, and health, and emit events from the MarginAccount primitives so owner escape-hatch actions are indexable. Regenerate gas snapshots.
Add forge-std as a top-level submodule at v1.16.2 and repoint the remapping off the transitive v4-core copy, for the EIP-7702 delegation cheatcodes used by the single-tx deploy.
Deploy and wire the whole stack in one EIP-7702 transaction: the deployer EOA delegates to a reusable BatchExecutor and self-calls a batch that deploys the account implementation, the three adapters, and the router through the standard CREATE2 factory (preserving the mined vanity address), then allowlists the adapters, registers markets, and proposes the final governance via the two-step handoff. A self-contained bootstrapper contract is impossible here: the stack's combined creation code (55.4KB) exceeds the EIP-3860 initcode limit (49.15KB).
Rename openPosition to increasePosition (and OpenParams to IncreaseParams, PositionOpened to PositionIncreased): the first call opens and subsequent calls add leverage, so 'increase' names the whole operation. Collapse closePosition into decreasePosition: passing debtToRepay == type(uint256).max fully closes (repay all by shares, withdraw all collateral, return residual PnL), otherwise a partial delever enforcing maxLtvAfter. Both modes share one implementation parameterized by the sentinel, and one PositionDecreased event carries the resulting state (a full close is signalled by debtTotal == 0, with collateralReturned the realized PnL); the separate PositionClosed event is removed. Migrate tests and regenerate snapshots.
The history rewrite reverted DeployMargin to the pre-fix constant and DeployMarginBootstrap carried the same stale oracle. That oracle hashes to an unlisted Morpho market with negligible liquidity, the root cause of the first position liquidation. Both scripts now register the canonical market and document the trap.
The original deployment was a test; its support is dropped rather than carried as a second contract entry. The new router's lifecycle events (PositionIncreased/PositionDecreased/CollateralAdded) emit equity, debt drawn, resulting totals, LTV, and health factor directly, so entry price, margin, and leverage no longer require same-tx lending-event correlation; the staged lending rows remain for venue attribution, liquidations, and escape-hatch flows. Close detection is now zero resulting totals on PositionDecreased. Schema gains router-reported state snapshots (lastLtvWad, lastHealthFactorWad, per-action ltvAfterWad/healthFactorWad).
4 participants
Margin Trading Periphery
Adds a periphery that opens leveraged spot positions in a single transaction by composing a Uniswap v4 swap with a borrow/supply against an external lending protocol. Three venues are integrated — Morpho Blue, Aave v3, and Aave v4 — all behind the same router; the caller selects the venue per call by passing the matching adapter. The whole leverage loop runs inside one
PoolManagerunlock using v4 flash accounting, so it nets to zero with no intermediate capital and no router-held residual.What a position is
Borrow the debt token → swap it into the collateral token (exact-output) → supply the collateral (equity + bought) → draw the debt to settle the swap. The result is a position that is long the collateral and short the debt, at a caller-chosen leverage bounded by the market's max LTV. Direction is set entirely by the
(collateral, debt)pairing — there is no separate flag. Long ETH (collateral WETH, debt USDC) and short ETH (collateral USDC, debt WETH) are both live today.Architecture
MarginRouterV4Router,ReentrancyLock,Permit2Forwarder,Multicall_v4,NativeWrapper, and the account factory. Is the trusted manager of every account.MarginAccountonBehalf == account), so it acts as itself with no delegated authorization. Owner and manager are baked into bytecode (soulbound: no initializer, no transfer). Borrowed and withdrawn funds are delivered to the account and forwarded to the validated receiver (measure-and-forward), so venues that lack a receiver argument are handled uniformly.MarginAccountFactoryMorphoLendingAdapter(collateral, debt) → MarketParamsrouting table (Morpho Blue).AaveLendingAdapter(collateral, debt)allowlist; resolves the Pool and protocol data provider from anIPoolAddressesProvider.AaveV4LendingAdapter(collateral, debt) → (collateralReserveId, debtReserveId)routes, validated on-chain at registration; batchessupply+setUsingAsCollateralin aSpoke.multicall; reads premium-inclusive debt.ILendingAdapter/IMarginAccount/IMarginRouterMarket(the pair + thetoSwapParamspool/market reconciliation choke point),Ltv,LeverageX18,MarketRegistry,Owner.Each adapter returns the
(target, value, callData)the account executes and holds no funds. Entry points:openPosition,increasePosition,addCollateral(payable; equity via Permit2 or native ETH),decreasePosition,closePosition, plusaccountOfand governance views.Key design decisions
executeescape hatch lets the owner always act directly on the lending protocol, so funds can't be trapped by router configuration.ILendingAdapter. Three venues ship today (Morpho Blue, Aave v3, Aave v4); additional protocols are added as new adapters with no router or account changes.borrowandwithdrawCollateraldeliver to the account and forward the measured delta to the validated receiver, because lending protocols differ on whether their borrow/withdraw expose a recipient (Morpho and Aave v3 do; Aave v4 sends tomsg.sender).encodeBorrowcarries no receiver, and the v3/Morpho withdraw paths are unaffected (their measured delta is zero).reserveIdrather than asset address; the adapter is one instance per Spoke (a second Spoke is a second deployment). Supply does not auto-collateralize, so it is batched withsetUsingAsCollateral; debt is drawn + accrued premium, read viagetUserTotalDebt; the v4 position-manager/intent apparatus is bypassed entirely because the account is its ownonBehalfOfand direct caller.0x30range, leaving0x1c–0x2freserved for future core v4 actions.maxDebtIn/maxCollateralIncaps as the binding slippage protection.minHopPriceX36is an optional secondary per-hop bound, enforced against the swap's realized output. Opens are all-or-nothing on amount: a v4 exact-output swap can partial-fill on a thin pool, soopenPosition/increasePositionassert the fullcollateralToBuywas delivered (a newASSERT_FILLmargin action) and revertIncompleteFillotherwise rather than opening a smaller position.Security review
Ran an 8-agent parallel review (attack-vector, math/precision, access-control, economic, execution-trace, invariant, periphery, first-principles) over the core router/account. The model held up: the transient active-account lifecycle is safe across multicall and revert, reentrancy is blocked, every flow nets its PoolManager deltas to zero, math/casts/rounding are correct, and account isolation holds. No permissionless fund-loss path was found. The Aave v3 and Aave v4 adapters reuse the same audited router and account unchanged (additive
ILendingAdapterimplementations) and are validated by full open/accrue/decrease/close lifecycle tests against the live mainnet deployments. A follow-up finding on v4 exact-output partial fills (Low: bounded by the input caps and the venue's own health check, no fund loss) was reproduced with a PoC and remediated. Remediations in this branch:closePositiontakes a swap-free withdraw path; zerocollateralToBuy/debtToRepayrejected with a clear errorclosePositionresidual was balance-basedOwnertypeV4Routernow prices the realized output, so an under-filled swap below the caller's bound revertsV4TooMuchRequestedPerHopSingle(exact-input and full fills unchanged)_openadds anASSERT_FILLaction that revertsIncompleteFillunless the swap delivered the fullcollateralToBuy(all-or-nothing).close/decreasealready revert fail-safe on under-fillTesting
Unit tests for all three adapters and the account/router; margin integration tests (Permit2, native ETH, reversed-decimal short); a regression suite for exact-output partial fills (realized-output price guard, all-or-nothing open, fail-safe close); and live mainnet-fork lifecycle tests: Morpho long (e2e, multi-borrower), Aave v3 short, Aave v4 short, plus hedge tests (cross-venue Morpho-long/Aave-short, same-venue, and shared-account-via-subId). Gas snapshots are committed in isolate mode.
Known limitations / follow-ups
New dependencies
Adds the
morpho-blueandsoladysubmodules. Aave v3 and Aave v4 are integrated via minimal vendored interfaces undersrc/interfaces/external/aaveandsrc/interfaces/external/aave-v4(no new submodule).