Conversation
Introduce per-stage claims via a validFrom timestamp encoded in the Merkle leaf, and a claimBatch entry point that claims multiple stages in a single transferFrom. Replace the per-address hasClaimed flag with a per-stage key keccak256(claimer, leaf).
|
/claude /codex |
Codex Code ReviewFindings
Actionable fix:
No other concrete security/bug/performance issues stood out in this diff. |
| ) | ||
| ); | ||
|
|
||
| bytes32 claimKey = keccak256(abi.encode(msg.sender, leaf)); |
There was a problem hiding this comment.
Low: redundant msg.sender in claim key
leaf is already keccak256(bytes.concat(keccak256(abi.encode(msg.sender, amount, validFrom)))), so it uniquely identifies (claimer, amount, validFrom). Re-hashing msg.sender with it adds a wasted keccak256 call without extra security.
| bytes32 claimKey = keccak256(abi.encode(msg.sender, leaf)); | |
| bytes32 claimKey = leaf; |
| require(success, "Failed to transfer funds"); | ||
|
|
||
| emit TokensClaimed(msg.sender, amount); | ||
| emit TokensClaimed(msg.sender, totalClaimable); |
There was a problem hiding this comment.
Low: batch emits a single event, losing per-stage auditability
Off-chain indexers (and future auditors) can only see the total amount transferred, not which individual stages were claimed. Consider emitting one TokensClaimed event per stage inside _verifyAndMark, or adding a separate StagesClaimed event that logs the stage details.
Review:
|
The leaf already encodes msg.sender in its preimage, making the extra keccak256(abi.encode(msg.sender, leaf)) wrapper redundant.
Summary
validFromin the Merkle leaf so each claim leaf represents a vesting stage gated by timestamp.claimBatchentry point to claim multiple stages in one tx with a singletransferFrom.mapping(address => bool) hasClaimedwith per-stage tracking keyed bykeccak256(claimer, leaf)._verifyAndMarkinternal helper shared byclaimandclaimBatch.Notes
claimnow takesvalidFrom) and to the Merkle leaf format. Any off-chain tree builder must be updated before this is deployed.hasClaimedkey type changes fromaddresstobytes32. Safe for a fresh deploy; not an in-place upgrade.