audit: claim contracts

[JuArce]

Summary

Addresses the findings from the two security audits of ClaimableAirdrop.sol (FuzzingLabs and Least Authority), both included under audits/claim_contracts/. All actionable findings are fixed in this single PR, and the contract now ships with a test suite.

Audit findings addressed

ID	Finding	Severity	Status
FL-AL-1	Ownership can be renounced, permanently disabling the airdrop	Low	✅ Fixed
FL-AL-2	transferFrom used without SafeERC20	Low	✅ Fixed
FL-AL-3	Centralization & operational dependencies	Info	⚠️ Documented (code side); multisig/timelock + monitoring is an ops decision
FL-AL-4	Compiler version outdated (bug fixed in 0.8.34)	Info	✅ Fixed
LA-1	Document edge cases (root-update revocation, first-come-first-served)	Info	✅ Fixed
LA-2	Implement a test suite	Info	✅ Fixed

Changes

Contract (src/ClaimableAirdrop.sol)

• FL-AL-1: added a renounceOwnership() override that reverts, matching AlignedToken, so the owner can't brick the airdrop by renouncing.
• FL-AL-2: import SafeERC20, add using SafeERC20 for IERC20, and replace both transferFrom + bool-check blocks in claim/claimBatch with safeTransferFrom. Robust to non-standard/no-return-data tokens (relevant since tokenProxy is an upgradeable proxy).
• FL-AL-3 / LA-1: contract-level NatSpec documenting the implicit edge cases — the owner can revoke unclaimed approvals via updateMerkleRoot, and claims are effectively first-come-first-served if the distributor is underfunded.

Compiler (FL-AL-4)

• Bumped pragma to 0.8.35 in ClaimableAirdrop.sol and AlignedToken.sol, and pinned solc_version = "0.8.35" in foundry.toml. Both contracts share dependencies and are imported together by the deploy scripts, so they must compile under a single version. The token is not being redeployed; the bump is a hygiene measure.

Test suite (LA-2)

• New test/ClaimableAirdrop.t.sol — 24 tests, covering: initialization, all claim revert branches (pause, deadline, vesting, double-claim, invalid proof, wrong amount, non-entitled caller), claimBatch (success, length mismatch, empty batch, duplicate leaf), access control (owner-only, requires-paused, renounce-reverts), and the SafeERC20 path (false-returning token + failed-transfer leaving the leaf claimable for retry).
• Merkle proofs come from the real generator, not Solidity. A small Rust binary (test/fixtures/generator/) uses the same merkle-tree-rs revision and leaf encoding as the production proof generator (aligned_airdrop_web/merkle_proof_generator) to produce test/fixtures/proofs.json. The tests load that fixture and verify the on-chain verifier against real generator output. The Rust toolchain is only needed to regenerate the fixture (see test/fixtures/README.md), never to run the tests.

Fix details (code links)

Permalinks point at the applied code as of commit 28a2a0b.

FL-AL-1 — Prevent ownership renouncement

• renounceOwnership() override that reverts: ClaimableAirdrop.sol L216-L221
• Test: test_renounceOwnership_reverts

FL-AL-2 — SafeERC20

• Import: ClaimableAirdrop.sol L5 · using directive: L31
• claim → safeTransferFrom: L109 · claimBatch: L142-L146
• Tests: test_claim_revertsWhenTokenReturnsFalse · test_claim_failedTransferDoesNotStrandLeaf

FL-AL-3 / LA-1 — Document edge cases

• Contract NatSpec on root-update revocation + first-come-first-served: ClaimableAirdrop.sol L15-L23

FL-AL-4 — Compiler bump to 0.8.35

• ClaimableAirdrop.sol L2 · AlignedToken.sol L2 · foundry.toml L8

LA-2 — Test suite

• test/ClaimableAirdrop.t.sol (24 tests) — multi-leaf coverage e.g. test_claim_fixtureProofs_allClaim, test_claimBatch_revertsOnDuplicateLeaf, test_claimBatch_revertsOnEmptyBatch
• Fixture generator (real merkle-tree-rs): test/fixtures/generator/src/main.rs → committed fixture test/fixtures/proofs.json

Notes for reviewers

• AlignedToken.sol was out of scope for the audits but is bumped to 0.8.35 to keep the project compiling under a single compiler (the deploy scripts import both contracts).
• FL-AL-3's operational recommendation (a multisig/timelock owner plus monitoring of the distributor's balance/allowance) is best addressed at the operational and deployment level rather than in the contract code, so we're tracking it separately and it isn't included in this PR.

Testing

• forge build --force — compiles cleanly on solc 0.8.35.
• forge test — 24 passed, 0 failed.

[github-actions]

Codex Code Review

No issues found in the PR diff.

I could not run the Foundry test suite because forge is not installed in this environment (forge: command not found). I did run git diff --check, which passed.

[claude]

Low — unpause() has no precondition guards

unpause() can be called when either limitTimestampToClaim == 0 or claimMerkleRoot == bytes32(0) (or both). In either case the contract appears live but every claim immediately reverts:

• limitTimestampToClaim == 0 → block.timestamp <= 0 is always false → "Drop is no longer claimable"
• claimMerkleRoot == bytes32(0) → MerkleProof.verify can never produce a zero root from a real leaf → "Invalid Merkle proof"

Recovery is always possible (the owner can re-pause, fix the missing precondition, and unpause again), but there is no on-chain warning. The intended call order — updateMerkleRoot, extendClaimPeriod, then unpause — is not enforced anywhere.

Suggested fix:

Suggested change

	_unpause();
	function unpause() external onlyOwner {
	require(claimMerkleRoot != bytes32(0), "Merkle root not set");
	require(limitTimestampToClaim > block.timestamp, "Claim period not set");
	_unpause();
	}

[claude]

Informational — unpinned dependencies with no Cargo.lock

ethers = "2" and serde_json = "1" are semver ranges and Cargo.lock is gitignored, so the fixture generator is not reproducible across environments. If a developer regenerates proofs.json after a breaking minor release of either crate, the build may silently succeed but produce a different fixture — causing opaque "Invalid Merkle proof" failures in the Solidity tests.

merkle-tree-rs is correctly pinned to a specific revision, which is the critical dependency. Consider either:

• Pinning ethers and serde_json to exact versions (ethers = "=2.0.14"), or
• Committing Cargo.lock for the generator binary (library crates conventionally omit it; binaries should include it for reproducibility).

[claude]

Review: audit/claim_contracts

This PR adds two audit reports and implements their recommendations: Solidity compiler bump (0.8.28 → 0.8.35), switching to SafeERC20 (replacing the manual bool success + require pattern), a renounceOwnership override that reverts, and a comprehensive Foundry test suite backed by a Rust-generated Merkle fixture. All three changes are meaningful security improvements. A few operational gaps remain.

---

Low — unpause() missing precondition guards (inline comment at line 213)

unpause() can be called when limitTimestampToClaim == 0 or claimMerkleRoot == bytes32(0). In either case the contract is live but every claim silently reverts with a misleading error ("Drop is no longer claimable" or "Invalid Merkle proof" respectively). Recovery requires a re-pause, fix, then re-unpause. Suggested fix in the inline comment: guard both conditions in unpause().

---

Low — hasClaimed persists across root updates; identical (address, amount, validFrom) cannot be re-allocated

hasClaimed is keyed on keccak256(keccak256(abi.encode(msg.sender, amount, validFrom))) and is never cleared when updateMerkleRoot replaces the root. If the new root re-issues the exact same (address, amount, validFrom) triple — e.g. the operator corrects a distribution while keeping the same amount and stage timestamp for a user who already claimed — that user's claim reverts on "Stage already claimed" with no recovery path short of a contract upgrade.

The new NatDoc block (lines 15–23) documents the revocation use case but not this inverse: re-allocating the same tuple after a root update is permanently blocked. A sentence in the NatDoc would prevent an operator footgun.

---

Informational — fixture generator reproducibility (inline comment in Cargo.toml)

ethers = "2" and serde_json = "1" are semver ranges and Cargo.lock is gitignored. Future regeneration of proofs.json is not guaranteed to build or produce identical output. The critical dependency (merkle-tree-rs) is correctly pinned to a specific git rev.

---

Not a bug — renounceOwnership() public view override

Compiles correctly. Solidity 0.8.x permits overriding a nonpayable parent with view (stricter mutability). The function always reverts so no state is written; the view modifier is valid and the ABI accurately reflects this.

[github-actions]

Codex Code Review

No issues found in the PR diff.

Static review completed. Tests could not be run because forge is unavailable in the environment.

[claude]

Review: audit fixes for ClaimableAirdrop.sol

Overview: Addresses FL-AL-1/2/3/4 and LA-1/2 from the two claim-contract audits: reverts renounceOwnership, migrates transferFrom to safeTransferFrom (SafeERC20), bumps solc to 0.8.35, adds NatSpec documenting operational edge cases, and adds a 24-test Foundry suite (with a Rust-generated Merkle fixture that mirrors the production proof generator).

Security

• No critical/high issues found. The nonReentrant guard was already in place on claim/claimBatch, so switching to safeTransferFrom is a clean drop-in with no reentrancy implications (checks-effects-interactions preserved: hasClaimed is marked before the transfer, and a failed transfer reverts the whole tx, so the leaf is never stranded — this is well covered by test_claim_failedTransferDoesNotStrandLeaf).
• renounceOwnership() public view override onlyOwner { revert(...); } correctly blocks renouncement while keeping the onlyOwner gate (redundant given the unconditional revert, but harmless).
• Compiler bump to 0.8.35 is applied consistently across ClaimableAirdrop.sol, AlignedToken.sol, and foundry.toml — no stale 0.8.28 references left behind.
• AlignedToken.sol's bump is a no-op for already-deployed bytecode since it isn't being redeployed, as noted in the PR description.

Bugs / correctness

• None found. Reviewed _verifyAndMark, claim, claimBatch, updateMerkleRoot, extendClaimPeriod — logic is unchanged aside from the SafeERC20 swap, and the new tests exercise the revert branches, batch edge cases (length mismatch, empty batch, duplicate leaf), and access control thoroughly.

Minor / nitpick

• claim_contracts/test/fixtures/generator/.gitignore excludes Cargo.lock for the fixture-generator crate. Since one dependency (ethers = "2") is not pinned to an exact version and isn't part of the byte-for-byte-critical merkle-tree-rs pin, regenerating the fixture months from now could pull a different transitive dependency tree. Low risk since the committed proofs.json fixture itself is what's actually tested against — this only matters if/when someone regenerates it. Consider committing Cargo.lock for reproducibility, though not blocking.

Test coverage

• Good breadth: init, all claim revert branches, batch claims (including a real multi-leaf/multi-proof fixture), access control, and the SafeERC20 failure path. Using proofs generated by the actual Rust generator (same merkle-tree-rs rev as production) instead of reimplementing Merkle logic in Solidity is a nice touch — it validates the on-chain verifier against real-world proof output rather than a Solidity-side mirror that could share the same bug.

Overall: solid, well-scoped fix with good test coverage. No blocking issues.