feat(invitations): cross-leg token exclusions to prevent batch double-spends#56
feat(invitations): cross-leg token exclusions to prevent batch double-spends#56leinss wants to merge 1 commit into
Conversation
…-spends When a Safe batch composes multiple independently path-found legs (e.g. legacy-CRC migration + invitation transfer), each pathfinder call plans against the same live balances, so both legs can source the same wrapped token. On-chain the first leg's unwrap consumes the balance and the second leg's unwrap reverts ERC20InsufficientBalance. Rebuilding on retry reproduces the same collision deterministically. - PermissionlessGroup.migration() returns sourcedTokens: the token owners the migration path spends from the avatar (wrapped entries resolved to the underlying avatar) - new getSourcedTokenOwnersFromPath() helper in sdk-pathfinder - Invitations.generateReferral()/generateInvite() accept options.excludeFromTokens, forwarded to findInvitePath()/ findFarmInvitePath() and into the pathfinder request (the server expands an excluded avatar to its wrapped forms) Additive only; no behavior change for existing callers.
Release Notes
WalkthroughThis PR adds double-spend prevention by threading an optional ChangesToken owner exclusion mechanism
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/invitations/src/Invitations.ts (1)
337-346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply
excludeFromTokensbefore choosing the proxy inviter.These branches still lock onto
realInviters[0], butgetRealInviters()is computed against the unexcluded state. If the first owner is inexcludeFromTokens,findInvitePath()fails immediately and we never try the next proxy inviter or the farm path, even though one may still satisfy the invite. The same gap exists infindInvitePath()whenproxyInviterAddressis omitted.Also applies to: 448-480, 706-716
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/invitations/src/Invitations.ts` around lines 337 - 346, The code picks realInviters[0] before applying options?.excludeFromTokens, causing findInvitePath(inviterLower, realInviterAddress, ...) to fail if that address is excluded; fix by filtering the realInviters list with options?.excludeFromTokens (same filter used by getRealInviters) before selecting a proxy and instead iterate through the filtered proxy candidates, calling findInvitePath(inviterLower, candidate.address, options?.excludeFromTokens) for each until one succeeds, and only then fall back to the farm path; also apply the same candidate-filter-and-iterate approach inside findInvitePath() when proxyInviterAddress is omitted to try each non-excluded proxy inviter rather than assuming the first entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/invitations/src/Invitations.ts`:
- Around line 337-346: The code picks realInviters[0] before applying
options?.excludeFromTokens, causing findInvitePath(inviterLower,
realInviterAddress, ...) to fail if that address is excluded; fix by filtering
the realInviters list with options?.excludeFromTokens (same filter used by
getRealInviters) before selecting a proxy and instead iterate through the
filtered proxy candidates, calling findInvitePath(inviterLower,
candidate.address, options?.excludeFromTokens) for each until one succeeds, and
only then fall back to the farm path; also apply the same
candidate-filter-and-iterate approach inside findInvitePath() when
proxyInviterAddress is omitted to try each non-excluded proxy inviter rather
than assuming the first entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f202f5cc-2c1a-4bb9-baae-4e16f67b028f
📒 Files selected for processing (4)
packages/invitations/src/Invitations.tspackages/pathfinder/src/path.tspackages/permissionless-groups/src/PermissionlessGroup.tspackages/permissionless-groups/src/types.ts
Summary
Composed Safe batches that contain multiple independently path-found legs (e.g. legacy-CRC migration + invitation transfer) can double-spend the same token balance: each leg's pathfinder call plans against the same live state, so both may source the same (wrapped) balance. On-chain, the first leg's
unwrapconsumes it and the second leg'sunwrapof the same amount revertsERC20InsufficientBalanceduring 4337 simulation. Rebuild-on-retry cannot fix this — rebuilding against unchanged chain state reproduces the same collision deterministically.Changes
PermissionlessGroup.migration()now returnssourcedTokens— the token owners (avatars) the migration path spends from the avatar, wrapped entries resolved to their underlying avatar.getSourcedTokenOwnersFromPath()helper in@aboutcircles/sdk-pathfinder.Invitations.generateReferral()/generateInvite()acceptoptions.excludeFromTokens, forwarded tofindInvitePath()/findFarmInvitePath()and into the pathfinder request.Additive only; no behavior change for existing callers.
Test plan
bun run build(tsc --build + all package builds) — greeneth_simulateV1; when exclusions make the target unreachable, the existing insufficient-balance error is thrown at build time instead of signing a doomed txexcludeFromTokens