BatchZap.sol

This document summarizes the security-focused review of BatchZap.sol, following a line-by-line and entry-point analysis. The contract provides batch operations for redeeming, selling back, burning, and transferring Slab NFTs in a single transaction. Findings are ordered by severity; each includes context, impact, and a recommended fix where applicable.

[HIGH] Missing Zero-Address Validation in batchTransfer()

The batchTransfer() function does not check that _recipient is not the zero address. Passing address(0) as the recipient can lead to tokens being sent to the zero address (or reverted, depending on the underlying ERC721 implementation), which may result in permanent loss of NFTs or confusing failures.

Recommended Solution

Add a check at the start of the function:

function batchTransfer(uint[] calldata _tokenIds, address _recipient) external whenNotPaused nonReentrant {
    if (_tokenIds.length == 0) {
        revert EmptyArray();
    }
    if (_recipient == address(0)) {
        revert InvalidRecipient(); // or a dedicated error, e.g. InvalidRecipient()
    }
    // ... rest of function
}

Define an InvalidRecipient error (or reuse an existing invalid-address error) and revert before the loop.

[HIGH] Missing Zero-Address Validation for SellBack.slabMachine

In batchSellBack(), each SellBack struct carries a slabMachine address. There is no validation that this address is non-zero. If a user (or integrator) passes address(0):

1. The contract calls slab.approve(address(0), sellBack.tokenId), which can clear approvals or behave in an implementation-dependent way.
2. The call ISlabMachine(address(0)).sellBack(sellBack.tokenId) will execute against no contract and the transaction will fail, but the intent to guard against invalid input is missing and the failure mode is unclear.

Validating slabMachine != address(0) makes the contract’s expectations explicit and avoids ambiguous behavior.

Recommended Solution

Inside the loop (or in a helper), before using sellBack.slabMachine:

if (sellBack.slabMachine == address(0)) {
    revert InvalidSlabMachine(); // add this error and use consistently for invalid machine address
}

[MEDIUM] Approval and Redemption Requirements Not Enforced or Documented

For batchRedeem() and batchBurn(), the contract checks that msg.sender (the user) owns each token via _assertOwner(), then calls slab.redeem() or IERC721Burnable(slab).burn(). In both cases, the caller of redeem/burn is the BatchZap, not the user. ERC721’s burn (and thus Slab’s redeem) requires the caller to be the owner or approved for the token. So the user must have approved the BatchZap for each token (or set the BatchZap as operator) before calling batchRedeem or batchBurn; otherwise the call will revert inside Slab with an authorization failure.

The same applies to batchTransfer(): the zap performs safeTransferFrom(msg.sender, _recipient, tokenId), so the zap must be approved (or operator) for the user’s tokens.

If this approval requirement is not clearly documented or surfaced in the UI, users may believe that ownership alone is enough and experience reverts without an obvious reason.

Recommended Solution

• In NatSpec and any integration docs, state explicitly that the user must approve the BatchZap (or set it as operator) for the relevant token IDs before calling batchRedeem, batchBurn, or batchTransfer.
• Optionally, add a view helper that returns the BatchZap address so front-ends can prompt for the correct approval.
• If desired, consider a pattern that pulls tokens into the zap first (e.g. transferFrom user → zap, then zap calls redeem/burn), so that a single approval of the zap is sufficient and the flow is consistent with batchSellBack (which does pull tokens to the zap first).

[MEDIUM] Unbounded Loops Over User-Controlled Arrays

All four batch functions—batchRedeem, batchSellBack, batchBurn, and batchTransfer—iterate over arrays whose length is chosen by the caller. A very large array can cause the transaction to run out of gas and revert. This does not lock funds, but it can be used to cause a denial of service for that transaction and may complicate integration (e.g. UI or bots that do not cap batch size).

Recommended Solution

Consider enforcing a maximum batch size so that gas stays predictable and integration is easier:

uint public constant MAX_BATCH_SIZE = 100; // or another protocol-chosen cap

function batchRedeem(Redemption[] calldata _redemptions) external whenNotPaused nonReentrant {
    if (_redemptions.length == 0) revert EmptyArray();
    if (_redemptions.length > MAX_BATCH_SIZE) revert BatchTooLarge();
    // ...
}

Apply the same pattern to batchSellBack, batchBurn, and batchTransfer. Document the limit and the rationale (gas and DoS mitigation).

[LOW] Duplicate tokenId in a Single Batch

In batchSellBack(), if the same tokenId appears in two or more SellBack entries (whether for the same or different machines), the first iteration will succeed: the token is transferred from the user to the zap, then to the machine. The second iteration will revert in _assertOwner(sellBack.tokenId, msg.sender) because the token is no longer owned by msg.sender. So there is no double-spend or loss of tokens, but the error is a generic NotOwner() and may be confusing if the user accidentally duplicated a token ID.

Recommended Solution

Either document that duplicate token IDs in one batch will revert with NotOwner, or add an explicit check (e.g. require that each token ID appears at most once in the batch) and revert with a clearer error such as DuplicateTokenId(uint tokenId).

[LOW] Compatibility with SlabMachine V2

BatchZap is written against ISlabMachine (V1). It calls sellBack(uint256) and then ISlabMachine(...).usdc().safeTransfer(msg.sender, buybackAmount), i.e. it expects the machine to hold USDC and send it to the zap. SlabMachineV2 uses the SlabLocker for custody and USDC; payouts go through the locker’s take() and the machine does not hold USDC. Therefore, BatchZap in its current form is not compatible with SlabMachineV2. Integrators and front-ends should only use this zap with V1 machines, or a separate zap (or updated interface) is needed for V2.

Recommended Solution

Document in the contract or protocol docs that BatchZap supports only SlabMachine (V1). If V2 support is required, introduce a separate zap or an adapter that uses the locker’s take() and does not assume machine.usdc().transfer(...) from the machine.

[INFORMATIONAL] Empty recipient in Redemption

batchRedeem() passes redemption.recipient (of type bytes) directly to slab.redeem(tokenId, recipient). The Slab contract does not appear to validate that recipient is non-empty; it only emits SlabRedeemed(_tokenId, _recipient) for off-chain processing. Whether empty recipient is allowed or desired is a product/design choice. If empty bytes are invalid for the redemption flow, validation should be added (in Slab or in the zap); otherwise, document that empty recipient is allowed and how it is handled off-chain.

Summary

Addressing the two high-severity items (zero-address checks) and clarifying or enforcing approval and batch-size behavior will significantly reduce risk and improve usability. The remaining items are good candidates for documentation and, where appropriate, small, backward-compatible improvements.