Off-by-one in finder.py drops token_id == totalSupply — owner can never claim

Bug Bounty
1

Severity: High
Component: ghost-preclaims / finder.py @ commit 1538567

Summary

finder.py scans the JML collection with range(start_token_id, total_supply). The collection is 1-indexed (token IDs 1..N), so the token with ID == totalSupply is never queried. Its owner never makes it into the snapshot, forester.py then publishes a Merkle root without them, and there is no way to claim afterwards. Re-running the tool doesn’t fix it — it just prints “All preclaims are already collected.”

The code

def load_existing_snapshot(file_path):
    ...
    return data, last_token_id + 1
    ...
    return [], 1   # fresh run starts at token 1

if start_token_id >= total_supply:
    print(f"[+] All preclaims are already collected.")
    sys.exit(0)

for token_id in tqdm(range(start_token_id, total_supply), desc=f"Scanning {chain_id}"):

range(1, total_supply) covers 1..N-1. Token N is skipped on every run.

Why this isn’t intended

I checked the actual JML collection on Sepolia (0x91ba8A14D2CC851aBb69212c09f59e06e1e7f0a5): ownerOf(0) reverts with ERC721: invalid token ID while ownerOf(1) returns an owner. So the collection is 1-indexed, the full range is [1, totalSupply], and the loop bound is just wrong. There is no indexing convention under which range(1, totalSupply) is complete.

Two more signs it’s a bug and not a cutoff rule:

The resume path stores last_token_id + 1, which only makes sense if the intent is to cover everything up to and including totalSupply.
With totalSupply == 1, a fresh run prints “All preclaims are already collected.” and exits without writing any snapshot file. That’s obviously not intended output.

Reproduction

Ran the repo’s own finder.py and forester.py unmodified against a local anvil chain, with a minimal mock of the JML interface (totalSupply/ownerOf/collateralBalances) deployed at the hardcoded address via anvil_setCode. Minting 1-indexed, 5 tokens (owners A1, A2, A3, A1, A1).

$ python3 finder.py --rpc http://127.0.0.1:8545 --block latest --delay 0
[+] Current totalSupply: 5
# snapshot token_ids: [1, 2, 3, 4]  -> token 5 (== totalSupply) never queried

$ python3 forester.py --network-id 31337
=== FINAL MERKLE ROOT [31337]: aa0a52991b768bacc3c2c2de4331a3db798e9242c074216430e7d97b3d88fe2e ===
# packages only carry token_ids [2, 3, 4] — no leaf/proof exists for token 5

$ python3 finder.py --rpc http://127.0.0.1:8545 --block latest --delay 0   # re-run
[+] All preclaims are already collected.
# exits immediately, token 5 still missing — cannot be repaired by re-running

And with totalSupply == 1: fresh run exits with “All preclaims are already collected.” without scanning the only token, and no snapshot file is written (the early exit sits before save_snapshot).

This already happened in the committed data

snapshots/137.json (Polygon) is missing token 2833 with no way to backfill it — the resume logic starts at max_token_id + 1, so the gap is permanent. Every committed snapshot tops out at totalSupply-1 of its snapshot block. So this isn’t theoretical: published snapshots are silently incomplete.

Impact

Whoever owns the highest-minted token at snapshot time gets no leaf in the Merkle tree, so no proof can ever be generated for them once the root is published. Their preclaim is permanently unclaimable. The progress bar ending at N-1 looks completely normal, so nothing alerts the operator.

Suggested fix

if start_token_id > total_supply:      # was >=

for token_id in tqdm(range(start_token_id, total_supply + 1), ...):   # was range(start_token_id, total_supply)

Note: existing snapshots are all short by exactly one token — fixing an already-published root means re-snapshotting and re-publishing that collection’s root

2

Thank you for your report!

First of all, if we’re talking about Sepolia, could you try running the script at a block number from roughly three weeks ago? There should be plenty of NFTs at that point. Or you could try other networks such as Gnosis or Polygon.

In any case, you’re right that there can be gaps between some token_ids. What do you think is a good way to avoid that while still distinguishing between an RPC error and a nonexistent token?

And you are absolutely on point with your suggested fix - in my view, you could continue this thread in order to elevate it down the line. You are absolutely on track, many thanks!

3

Thanks for the suggestion. I checked the same JML collection on Polygon mainnet at historical block 93700000 (2026-09-12 23:17:13 UTC)

At that block:

- totalSupply() = 65892
- ownerOf(2832) -> 0xBc1ACF225c4Fc99D758C98Ea84fb29FD75Ee1e5e
- ownerOf(2833) -> reverts with ERC721: invalid token ID
- ownerOf(2834) -> 0xe93a577768fb2b98195EDdD3c3a20D8a8328eF5d

So there is a real gap: 2832 exists, 2833 does not, and 2834 exists.

I then ran the unmodified finder.py against the same historical block, starting from a snapshot whose last processed token was 2832.

The first run stopped at 2833:

[+] Snapshot target block: 93700000
[+] Snapshot exists. Last processed Token ID: 2832
[-] Error occured during on the Token ID 2833:
    execution reverted: ERC721: invalid token ID
[+] Saved to file: snapshots\137.json
[+] Chain [137] has 1 records

I ran it again without changing the source or snapshot. It stopped at exactly the same token again:

[+] Snapshot target block: 93700000
[+] Snapshot exists. Last processed Token ID: 2832
[-] Error occured during on the Token ID 2833:
    execution reverted: ERC721: invalid token ID
[+] Saved to file: snapshots\137.json
[+] Chain [137] has 1 records

So the gap does not just cause token 2833 to be skipped. Because ownerOf() errors are handled by the generic exception path and the scanner executes break, the scanner never reaches the valid token 2834 or any later token. Re-running with the same snapshot state hits the same gap again.

For the error-handling question, I think the important distinction is:

  • a confirmed nonexistent token should be recorded/skipped so scanning can continue;
  • a transient RPC/provider failure should be retried or stop the scan without advancing past that token.

A longer-term solution could avoid assuming that token IDs are contiguous altogether, for example by enumerating the existing token set directly where the collection supports it.

I think this confirms the gap/resume issue on a real network and historical state.

4

That’s the catch!

How would you differentiate an ownerOf error from any other RPC error? The main goal is to make it runnable for everyone in the same manner. Any ideas here?

By the way, there is a dirty trick - though I personally don’t like it at all: add an additional CLI flag such as --excluded-token-ids, which would list all token IDs that need to be excluded during the process loop.

5

That’s a fair point. I don’t think every ownerOf() revert should be treated as a missing token either.

For this case, I think the scanner can check the revert reason/data for the known nonexistent-token case and only skip when it matches. Other contract errors shouldn’t be skipped automatically.

For actual RPC/provider failures, retrying while keeping the resume state unchanged would avoid moving past an unprocessed token.

The pinned --block helps here too, since the same on-chain revert should be reproducible. In my case, 2833 returned the same ERC721: invalid token ID on repeated runs at block 93700000, while 2834 was valid.

So I’d prefer automatic detection over --excluded-token-ids, since the scanner shouldn’t need to know the gaps beforehand.

6

Can you take a look at the other topic plase. The authhor there is proposing to parse errors during execution to distinguish RPC failures from empty responses to ownerOf. Can you confirm whether the linked topic is fixing the same issue in the way described?

7

I took a look at BYFOR’s topic. Yeah, I think it’s fixing the same gap/truncation issue I reproduced in my follow-up. The big change is that the scanner doesn’t stop on the bad token anymore, so it can skip it and keep going to the next valid ID. They also added retries, block pinning, and a completeness check.

The one thing I’d handle a bit differently is the skip condition. I’d only skip when the error actually shows that the token doesn’t exist. Otherwise, another contract error could get treated as a missing token.

So yes, I think their patch covers the 2833 → 2834 case from my PoC. The original token_id == totalSupply off-by-one is still separate though, since they left range(start_token_id, total_supply) unchanged.