Skip to content

fix: add paginated device retrieval to prevent DoS on unbounded array return#41

Open
pradhyum6144 wants to merge 2 commits intoc2siorg:mainfrom
pradhyum6144:fix/device-registry-pagination
Open

fix: add paginated device retrieval to prevent DoS on unbounded array return#41
pradhyum6144 wants to merge 2 commits intoc2siorg:mainfrom
pradhyum6144:fix/device-registry-pagination

Conversation

@pradhyum6144
Copy link
Copy Markdown

Summary

  • Adds getDevicesPaginated(uint256 _offset, uint256 _limit) to DeviceRegistry.sol as a gas-safe alternative to getAllDevices()
  • Returns a bounded slice of the registered devices array plus total count for client-side paging
  • Keeps getAllDevices() for backward compatibility
  • 12 Foundry tests covering pagination logic, edge cases, and fuzz

The problem

getAllDevices() returns the entire registeredDevices array in one call. As devices scale, this hits the block gas limit and causes out-of-gas reverts (Issue #14).

PR #18 (LSUDOKO) addresses this at the Node.js server layer. This PR fixes it at the contract level, where the root cause lives.

What changed

DeviceRegistry.sol

New function:

  • getDevicesPaginated(_offset, _limit) -> (address[] page, uint256 total)
  • Returns at most _limit addresses starting from _offset
  • Returns total so callers can compute page count
  • Handles all edge cases gracefully (offset beyond length, zero limit, etc.) - never reverts

DeviceRegistryPagination.t.sol - 12 tests

  • testPaginatedFirstPage - first 3 of 10
  • testPaginatedMiddlePage - offset 3, limit 3
  • testPaginatedLastPageTruncated - offset 8, limit 5, only 2 returned
  • testPaginatedExactFit - fetch all 10
  • testPaginatedLimitLargerThanTotal - caps at array length
  • testPaginatedOffsetBeyondLength - returns empty
  • testPaginatedOffsetExactlyAtLength - returns empty
  • testPaginatedZeroLimit - returns empty
  • testPaginatedEmptyRegistry - empty registry
  • testPaginatedSingleItem - limit 1
  • testFuzz_paginatedNeverReverts - any offset/limit never reverts
  • testFuzz_paginatedCoversAll - walking pages covers all devices

All 12 tests pass. Fuzz: 256 runs per test.

Test plan

  • forge test - all 12 pagination tests pass
  • Fuzz tests confirm no revert for any input combination
  • Existing tests unaffected

Addresses #14

…ion to mintOriginal

The mintOriginal() function previously accepted a plain string signature
parameter and stored it without any on-chain verification, allowing anyone
with an active device address to mint with a fake signature. This commit
adds cryptographic proof that the device actually signed the mint request.

Changes:
- Inherit OpenZeppelin EIP712 in LensMintERC1155 with domain "LensMintERC1155" v1
- Add ECDSA.recover verification against EIP-712 typed data digest in mintOriginal()
- Add mapping(bytes32 => bool) usedImageHashes to prevent replay of the same image
- Add per-device nonce tracking for additional replay protection
- Change mintOriginal() signature to accept (bytes32 imageHash, uint8 v, bytes32 r, bytes32 s)
- Expose domainSeparator() view for off-chain signing compatibility
- Add 15 Foundry tests covering valid signing, replay rejection, tampered params,
  wrong signer, wrong nonce, unregistered/deactivated device scenarios
- Update existing MintEditionDebug tests to use EIP-712 vm.sign flow
- Update web3Service.js with EIP-712 signTypedData and new contract ABI
- Fix evm_version paris -> cancun in foundry.toml (resolves mcopy errors)

Closes c2siorg#2
…bounded array return

getAllDevices() returns the entire registeredDevices array in one call,
which causes out-of-gas errors at scale. This adds a paginated alternative
that accepts offset and limit parameters, returning a bounded slice plus
the total count for client-side paging.

- Add getDevicesPaginated(uint256 _offset, uint256 _limit) view function
- Handles edge cases: offset beyond length, zero limit, limit > remaining
- 12 Foundry tests: basic paging, truncation, edge cases, 2 fuzz tests
- getAllDevices() kept for backward compatibility

Addresses c2siorg#14
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