[#893] Add finalize script and MerkleClaim contract#911
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
This PR is not merge-ready yet. CI is broken by an out-of-sync lockfile, and the finalize script does not guarantee the distributed amounts sum exactly to the intended pool slice.
Findings
- [high]
package.jsonaddsutf-8-validate, butpackage-lock.jsondoes not include a matching locked entry, sonpm cifails before any tests run.- File:
package.json:36 - Suggestion: Regenerate and commit
package-lock.jsonso it is fully in sync with the dependency manifest.
- File:
- [high] The distribution code rounds each recipient independently with
parseUnits(plotAmount.toFixed(18), 18). That can leave leftover wei undistributed or over-distribute by cumulative rounding, which violates issue #893's requirement that distribution amounts sum exactly topool_amount × milestone_pct.- File:
scripts/airdrop-finalize.ts:114 - Suggestion: Compute allocations in integer wei from a single total distributable amount, then assign the remainder deterministically (for example to the largest holder or via largest-remainder ordering) so the final sum is exact.
- File:
Decision
Requesting changes because the PR currently fails CI and the finalize script can produce a payout set that does not match the exact distributable amount.
…nc lockfile Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The rounding approach changed, but the PR is still blocked by the same lockfile issue in CI.
Findings
- [high]
npm ciis still failing in GitHub Actions becausepackage.jsonandpackage-lock.jsonare not actually in sync. The failed run still reportsMissing: utf-8-validate@5.0.10 from lock file, so this PR cannot pass install/CI as submitted.- File:
package-lock.json:1 - Suggestion: Re-run
npm install(or otherwise regenerate the lockfile cleanly), commit the resultingpackage-lock.json, and verifynpm cisucceeds before re-requesting review.
- File:
Decision
Requesting changes because the PR remains red in CI on a dependency-lock mismatch.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The bigint distribution update addresses the allocation-precision concern, but the PR is still blocked by the same dependency-lock mismatch in CI.
Findings
- [high] GitHub Actions is still failing at
npm ciwithMissing: utf-8-validate@5.0.10 from lock file, sopackage.jsonandpackage-lock.jsonremain out of sync in the submitted branch.- File:
package-lock.json:1 - Suggestion: Regenerate the lockfile cleanly and confirm
npm cipasses before re-requesting review.
- File:
Decision
Requesting changes because the PR remains red in CI on the install step.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The package manifest change moved in the right direction, but the PR is still blocked by the same install failure in CI.
Findings
- [high] GitHub Actions is still failing at
npm ciwithMissing: utf-8-validate@5.0.10 from lock file, so the submittedpackage-lock.jsonstill does not satisfy the dependency graph npm expects for this branch.- File:
package-lock.json:1 - Suggestion: Regenerate the lockfile from a clean install state and verify
npm cipasses before re-requesting review.
- File:
Decision
Requesting changes because the PR remains red on the install step.
…atch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The latest update resolves the lockfile blocker from my prior reviews. The regenerated package-lock.json is consistent with the manifest changes, and the bigint allocation logic remains the correct approach for exact distribution totals.
Findings
- No remaining code-level blockers in the changed files.
Decision
Approving because the finalize script and Merkle claim flow I reviewed now satisfy issue #893. The latest GitHub snapshot had CI pending rather than failing immediately on npm ci, so my prior merge blocker is resolved.
Fixes #893
Summary
pl_daily_pricesbefore campaign end@openzeppelin/merkle-treewith per-user proofspl_airdrop_proofsDB table andscripts/airdrop-proofs.jsonkeccak256(bytes.concat(keccak256(abi.encode(...))))) matching StandardMerkleTreeClaimedevent@openzeppelin/merkle-treedependencyTest plan
pool × milestone_pct🤖 Generated with Claude Code