-
Notifications
You must be signed in to change notification settings - Fork 59
Ai review project 60days #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Executive SummaryThis change introduces a new token launch mechanism ( Current Implementation Scope: Only the tax recipient update mechanism is implemented. The "rug" and "commit" drain logic is NOT included in this change. Product Context60 Days Launch Mechanism OverviewThe system introduces a new project type with a 60-day commitment window:
Note: The current implementation only handles tax recipient redirection. Drain logic for commit/rug scenarios is NOT implemented. Please help review |
AI-Reviewer-QS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these findings affect event data integrity with no direct fund risk. Recommend fixing before mainnet deployment to ensure reliable off-chain indexing and audit trails.
And another similar issue, which cannot be added to the review due to GitHub limit:
File: contracts/launchpadv2/BondingV2.sol
Lines: 401-408
_token.initialPurchase is set to 0 on line 401, then emitted on line 408. The event will always log 0 as the refund amount regardless of actual amount transferred.
Why this happens:
| Step | _token.initialPurchase |
|---|---|
| Line 395-398: Transfer | Correct value used ✅ |
| Line 401: Reset | Set to 0 |
| Line 408: Emit | 0 emitted ❌ |
Impact: Off-chain indexers/analytics will record all cancelled launches as having 0 refund. No fund loss—the actual transfer on line 395-398 uses the correct pre-zeroed value.
Fix: Cache the value before zeroing:
uint256 refundAmount = _token.initialPurchase;
_token.initialPurchase = 0;
_token.launchExecuted = true;
emit CancelledLaunch(_tokenAddress, _token.pair, tokenInfo[_tokenAddress].virtualId, refundAmount);| TaxRecipient storage recipient = _agentRecipients[agentId]; | ||
| address oldCreator = recipient.creator; | ||
| recipient.tba = tba; | ||
| recipient.creator = creator; | ||
| emit CreatorUpdated(agentId, oldCreator, creator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If _agentRecipients[agentId] was never accessed before, recipient.creator defaults to address(0). The actual original creator is available via info.founder (line 370), but it's not used to initialize the storage.
Compare with _getTaxRecipient() pattern:
if (recipient.tba == address(0)) {
recipient.tba = info.tba;
recipient.creator = info.founder; // ← Correct initialization
}Impact: First-time updates for any agentId will emit oldCreator = 0x0000... instead of the actual founder address, breaking historical tracking.
Fix: Apply the same initialization pattern:
TaxRecipient storage recipient = _agentRecipients[agentId];
if (recipient.tba == address(0)) {
recipient.tba = info.tba;
recipient.creator = info.founder;
}
address oldCreator = recipient.creator;
recipient.tba = tba;
recipient.creator = creator;
emit CreatorUpdated(agentId, oldCreator, creator);
| TaxRecipient storage recipient = _agentRecipients[agentId]; | ||
| recipient.tba = tba; | ||
| recipient.creator = creator; | ||
| emit CreatorUpdated(agentId, tba, creator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The emitted event uses tba as the second parameter, but CreatorUpdated is defined as:
event CreatorUpdated(
uint256 agentId,
address oldCreator, // ← Expected: previous creator address
address newCreator
);Impact: Off-chain systems parsing CreatorUpdated events will interpret the TBA address as the old creator, corrupting audit trails and making creator change history unreliable.
Fix: Capture old creator before overwriting:
TaxRecipient storage recipient = _agentRecipients[agentId];
address oldCreator = recipient.creator;
recipient.tba = tba;
recipient.creator = creator;
emit CreatorUpdated(agentId, oldCreator, creator);
No description provided.