Audit of Lighter's Spot Market Circuits and Multi-Asset Support

Description. The L1Deposit transaction supports two route types: SPOT (credits to asset balance) and PERPS (credits to collateral). When using the PERPS route, the circuit does not validate that the deposited asset is USDC or has margin_mode enabled.

In l1_deposit.rs lines 195-235, the apply() function determines the route and credits collateral without any asset validation:

let is_perps = builder.is_equal_constant(self.route_type, ROUTE_TYPE_PERPS as u64);

let extended_balance_delta_biguint = builder.mul_biguint_non_carry(
    &self.accepted_amount,
    &tx_state.assets[TX_ASSET_ID].extension_multiplier,
    BIG_U96_LIMBS,
);

// ...

let should_update_perps_balance = builder.and(should_update_balance, is_perps);
tx_state.accounts[OWNER_ACCOUNT_ID].collateral = builder.select_bigint(
    should_update_perps_balance,
    &BigIntTarget { abs: trimmed_abs_collateral_after, sign: balance_after.sign },
    &tx_state.accounts[OWNER_ACCOUNT_ID].collateral,
);

The collateral field is an USDC asset (as evidenced by USDC_TO_COLLATERAL_MULTIPLIER usage throughout the codebase). However, the circuit allows any asset to be deposited via the PERPS route and credited to collateral using that asset’s extension_multiplier.

Compare this to l2_transfer.rs lines 206-218, which correctly validates both margin_mode and is_usdc_asset for PERPS routes:

let is_asset_margin_enabled = builder.is_equal_constant(
    tx_state.assets[TX_ASSET_ID].margin_mode,
    ASSET_MARGIN_MODE_ENABLED,
);
// ...
let is_invalid_route_type = builder.and_not(is_perps, is_asset_margin_enabled);
builder.conditional_assert_false(is_enabled, is_invalid_route_type);

let is_to_perps_invalid_route = builder.and_not(is_perps, is_usdc_asset);
builder.conditional_assert_false(self.success, is_to_perps_invalid_route);

Impact. An attacker can inflate their USDC-denominated collateral by depositing a low-value token via the PERPS route:

1. Attacker deposits 1000 units of cheap token X via L1 with route_type=PERPS
2. Circuit credits 1000 * X.extension_multiplier to their collateral
3. Attacker now has inflated USDC collateral without depositing actual USDC

This inflated collateral can then be used to withdraw real USDC via the PERPS route, or to take leveraged positions beyond the attacker’s actual capital.

Recommendation. Add validation in L1Deposit that when route_type is PERPS, the asset_index must be USDC_ASSET_INDEX (or alternatively, verify margin_mode is enabled). This should mirror the validation present in l2_transfer.rs.

Client response. Fixed in commit b31b173 (Phase 2), refined in commit f6818da. The fix now checks that margin_mode is enabled for the asset when using the PERPS route with non-zero accepted amount, rather than hardcoding a check for USDC_ASSET_INDEX. Note that currently only USDC has margin_mode enabled. If margin mode is enabled for additional assets in the future while collateral remains USDC-denominated, this check would need to be revisited to prevent the same inflation issue.

Description. Transaction processing constraints do not validate that asset_index values fall within the valid range of 1 to 62. The system reserves indices 0 and 63 as invalid, but this is not enforced at the circuit level.

In tx_constraints.rs, asset indices are created as virtual targets without range constraints:

asset_indices: core::array::from_fn(|_| builder.add_virtual_target()),

These indices are subsequently used to derive Merkle paths and access asset data, but are never checked against MIN_ASSET_INDEX (1) or MAX_ASSET_INDEX (62).

This becomes problematic due to an asymmetry in the delta circuit. In account_delta_full_leaf.rs lines 143-166, the asset delta root calculation iterates over all 64 indices:

pub fn get_asset_delta_root(&self, builder: &mut Builder) -> HashOutTarget {
    let mut level_hashes = self
        .aggregated_asset_deltas
        .iter()  // All 64 elements (0-63)
        .map(|a| {
            // ... hash each element
        })
        .collect::<Vec<_>>();
    // ... builds merkle tree from all 64
}

However, in delta_constraints.rs lines 174-193, the public digest computation only includes indices 1-62:

for i in MIN_ASSET_INDEX as usize..=MAX_ASSET_INDEX as usize {
    let is_asset_empty = builder.is_zero_bigint(&delta.aggregated_asset_deltas[i]);
    // ...
}

Indices 0 and 63 are neither checked for emptiness nor included in the public digest, yet they contribute to the asset delta root and thus the state root.

Impact. An attacker can create transactions targeting asset indices 0 or 63. Since the circuit does not enforce range constraints, such transactions will:

1. Pass circuit verification (the user signs a valid transaction, just with an out-of-range index)
2. Affect the state root (via asset delta root which includes all 64 indices)
3. Not be reflected in the public digest (which only covers indices 1-62)

This corrupts data availability: the published pubdata no longer represents the complete state. Users relying on the escape hatch mechanism cannot reconstruct the full state from pubdata, potentially preventing emergency withdrawals.

This attack does not require a malicious prover or operator; any user can sign a transaction with an invalid asset index, and the sequencer will process it since the circuit accepts it.

Recommendation. Add range validation for asset_index in transaction constraints to ensure values fall within MIN_ASSET_INDEX to MAX_ASSET_INDEX. This is the primary fix as it prevents invalid transactions from being processed.

As defense in depth, consider also enforcing in the delta circuit that asset deltas at indices 0 and 63 must be zero, ensuring consistency between the asset delta root and the public digest.

Client response. Fixed in commit b31b173 (Phase 2). Added ensure_valid_asset_index checks in relevant transactions that reject asset indices 0 and 63 (i.e., MIN_ASSET_INDEX - 1 and MAX_ASSET_INDEX + 1).

Description. The is_valid_base_size_and_price function contains a logic error that inverts the check for the maximum order quote amount.

The code calculates the boolean flag quote_gt_max_quote_amount as follows:

let quote_gt_max_quote_amount = builder.is_lt_biguint(&quote_big, &max_quote_big);

This expression evaluates to true when quote_big is strictly less than max_quote_big. However, the variable name and subsequent logic imply it should be true when the quote amount exceeds the maximum limit.

Later in the function, this flag is used to invalidate the order:

let should_be_false = builder.or(should_be_false, quote_gt_max_quote_amount);
builder.not(should_be_false)

Because quote_gt_max_quote_amount is true for valid orders (those within the limit), the circuit incorrectly rejects them. Conversely, it accepts orders that are greater than or equal to the maximum limit (where is_lt_biguint returns false).

Recommendation. Correct the comparison to check if the quote amount is greater than the maximum limit. This can be done by replacing is_lt_biguint with is_gt_biguint.

Client response. Fixed in commit f6818da on the spot branch. Changed is_lt_biguint to is_gt_biguint as recommended.

Description. The L1UpdateAsset transaction allows updating asset configuration parameters including margin_mode. However, the circuit does not enforce the invariant that only USDC (the collateral asset) should have margin_mode set to ENABLED.

In l1_update_asset.rs lines 83-104, the verify() function only checks that margin_mode is a boolean value:

builder.assert_bool(BoolTarget::new_unsafe(self.margin_mode));

It does not verify that if margin_mode is being set to ENABLED, the asset must be USDC. Compare this to the correct implementation in l1_register_asset.rs lines 125-130:

let is_margin_enabled = builder.is_equal_constant(self.margin_mode, ASSET_MARGIN_MODE_ENABLED);
let is_usdc_asset = builder.is_equal_constant(self.asset_index, USDC_ASSET_INDEX);
let should_be_false = builder.and_not(is_margin_enabled, is_usdc_asset);
self.success = builder.and_not(self.success, should_be_false);

The L1RegisterAsset correctly enforces that enabling margin mode on a non-USDC asset causes the transaction to fail. This same constraint is missing from L1UpdateAsset.

Impact. An operator could enable margin_mode on a non-USDC asset, breaking a protocol invariant. Several transaction types rely on the margin_mode check as a guard for PERPS route operations (e.g., l1_withdraw.rs, l2_withdraw.rs). If this invariant is violated, those guards become ineffective, potentially allowing non-USDC assets to interact with collateral incorrectly.

The severity is reduced because exploiting this requires operator action (L1 priority operation), and impact depends on other transaction behaviors. However, it represents a defense-in-depth failure that could become critical if combined with other issues.

Recommendation. Add the same constraint present in L1RegisterAsset to L1UpdateAsset. Specifically, when margin_mode is set to ENABLED, the circuit should verify that asset_index equals USDC_ASSET_INDEX and fail the transaction otherwise.

Client response. Fixed in commit b31b173 (Phase 2). Added the same constraint from L1RegisterAsset: if margin_mode is enabled on a non-USDC asset, the transaction now fails.

Description. The asset management transactions have minor inconsistencies in asset index validation:

1. l1_update_asset.rs does not check that asset_index falls within MIN_ASSET_INDEX to MAX_ASSET_INDEX. Currently this is mitigated because assets must be non-empty to be updatable, and assets cannot be registered outside the valid range. However, adding an explicit range check would provide defense in depth.

2. l1_register_asset.rs checks for invalid indices using builder.is_zero(self.asset_index) instead of comparing against MIN_ASSET_INDEX. While functionally equivalent (since MIN_ASSET_INDEX = 1), using the constant would be more consistent and self-documenting.

Recommendation. Add explicit range validation in l1_update_asset.rs and replace the hardcoded zero check in l1_register_asset.rs with MIN_ASSET_INDEX for consistency.

Client response. Fixed in commit b31b173 (Phase 2). Both transactions now use ensure_valid_asset_index which checks against MIN_ASSET_INDEX and MAX_ASSET_INDEX constants.

Description. The nonzero_asset_count calculation in the delta circuit is off by two due to a mismatch between the loop iteration range and the total asset list size.

The circuit computes the count of non-zero asset deltas as follows:

let mut zero_asset_count = builder.zero();
for i in MIN_ASSET_INDEX as usize..=MAX_ASSET_INDEX as usize {  // 1..=62
    let is_asset_empty = builder.is_zero_bigint(&delta.aggregated_asset_deltas[i]);
    zero_asset_count = builder.add(zero_asset_count, is_asset_empty.target);
}
let total_asset_count = builder.constant_usize(ASSET_LIST_SIZE);  // 64
let nonzero_asset_count = builder.sub(total_asset_count, zero_asset_count);

The loop only iterates over indices 1 through 62 (the valid asset range, as indices 0 and 63 are reserved), counting at most 62 zero assets. However, total_asset_count was set to 64 (the full ASSET_LIST_SIZE). This meant nonzero_asset_count is always 2 higher than the actual count of non-zero assets.

For example, if all 62 valid assets had zero deltas, zero_asset_count would be 62, and nonzero_asset_count would compute to 64 - 62 = 2 instead of the correct value of 0.

This value is included in the delta digest used for public data reconstruction. If the off-chain reconstruction code uses the same logic, the system remains internally consistent. However, the semantic meaning of the field would be incorrect.

Recommendation. Initialize zero_asset_count to 2 to account for the reserved indices 0 and 63, or adjust total_asset_count to 62 to match the iteration range.

Client response. Fixed in commit b31b173 (Phase 2) by initializing zero_asset_count to builder.two().