feat: withdraw agent-wallet funds and pay the agent a commission#1389
Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
|
@holdex pr submit-time 4h |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a complete MAA withdrawal feature enabling agents to withdraw and bridge funds with commission handling. The core migration defines bridge dispatch helpers, a two-leg atomic settlement that transfers commission first then routes payout internally or to L1, and public entry points. A production migration override and end-to-end test suite validate the implementation. ChangesMAA Withdrawal Feature
Sequence DiagramsequenceDiagram
participant Caller as MAA Caller
participant WithdrawAPI as maa_withdraw / maa_bridge_out
participant DoWithdraw as maa_do_withdraw
participant Ledger as Token Ledger
participant Events as Event Log
Caller->>WithdrawAPI: invoke with bridge and amount
WithdrawAPI->>DoWithdraw: delegate with offramp flag
DoWithdraw->>Ledger: resolve MAA instance and fee terms
DoWithdraw->>Ledger: check free balance >= amount
DoWithdraw->>Ledger: compute commission (flat or bps)
DoWithdraw->>Ledger: transfer commission to restricted address
alt offramp = false
DoWithdraw->>Ledger: transfer payout to owner
else offramp = true
DoWithdraw->>Ledger: bridge payout to L1 recipient
end
DoWithdraw->>Events: record WITHDRAW event
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/streams/maa/withdraw_test.go (1)
123-134: ⚡ Quick winMake WITHDRAW event assertion deterministic and rule-local.
This assertion is currently coupled to
goldenRuleIDHexand to “last non-nil amount” across all rows. Prefer asserting against the created test rule ID and reading amount only from theWITHDRAWrow.💡 Proposed refactor
-func setupFundedMAA( +func setupFundedMAA( t *testing.T, ctx context.Context, platform *kwilTesting.Platform, restricted, unrestricted util.EthereumAddress, feeBps int64, salt []byte, fund string, -) []byte { +) ([]byte, []byte) { t.Helper() require.NoError(t, erc20bridge.ForTestingInitializeExtension(ctx, platform)) ruleID := createDefaultRule(t, ctx, platform, restricted, feeBps, salt) maa := joinRule(t, ctx, platform, unrestricted, ruleID) orderedsync.ForTestingReset() maaAddr := addrFromBytes(maa) fundAddress(t, ctx, platform, maaAddr.Address(), fund, 1) - return maa + return maa, ruleID }- maa := setupFundedMAA(t, ctx, platform, restricted, unrestricted, 250, repeat(0xab, 32), "100000000000000000000") + maa, ruleID := setupFundedMAA(t, ctx, platform, restricted, unrestricted, 250, repeat(0xab, 32), "100000000000000000000") maaAddr := addrFromBytes(maa) ... - var evtTypes []string - var lastAmount string + var evtTypes []string + var withdrawAmount string require.NoError(t, callAs(ctx, platform, restricted, "maa_get_events", []any{ - func() []byte { b, _ := hex.DecodeString(goldenRuleIDHex); return b }(), int64(100), int64(0), + ruleID, int64(100), int64(0), }, func(row *common.Row) error { - evtTypes = append(evtTypes, row.Values[2].(string)) - if row.Values[7] != nil { - lastAmount = row.Values[7].(*kwilTypes.Decimal).String() + eventType := row.Values[2].(string) + evtTypes = append(evtTypes, eventType) + if eventType == "WITHDRAW" && row.Values[7] != nil { + withdrawAmount = row.Values[7].(*kwilTypes.Decimal).String() } return nil })) require.Contains(t, evtTypes, "WITHDRAW") - require.Equal(t, "100000000000000000000", lastAmount, "WITHDRAW records the gross amount") + require.Equal(t, "100000000000000000000", withdrawAmount, "WITHDRAW records the gross amount")🤖 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 `@tests/streams/maa/withdraw_test.go` around lines 123 - 134, The test currently checks for a WITHDRAW event using the global goldenRuleIDHex and captures the last non-nil amount across all rows; change it to use the test's created rule ID (the local variable that holds the rule ID you create in this test) instead of goldenRuleIDHex and, inside the callAs row callback (used by maa_get_events), only read and assert lastAmount when the row corresponds to the WITHDRAW event (e.g., check row.Values[...] == "WITHDRAW" using the same index used for evtTypes or match the rule ID column to the created rule ID) so the amount is taken from that specific WITHDRAW row and the assertion becomes deterministic and rule-local.
🤖 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.
Inline comments:
In `@internal/migrations/049-maa-funding.sql`:
- Around line 127-153: In maa_do_withdraw, after the for-loop that reads
maa_rules for $rule_id, add an explicit check that a row was found (e.g. test
$restricted_bytes IS NOT NULL or another field) and raise an ERROR if not found,
and also validate $fee_mode is non-NULL and one of the allowed values before
calling maa_commission; ensure these checks occur before computing $commission
and $payout so you never reach event recording with NULL fee/payee fields
(referencing symbols: maa_do_withdraw, maa_rules, $restricted_bytes, $fee_mode,
maa_commission).
---
Nitpick comments:
In `@tests/streams/maa/withdraw_test.go`:
- Around line 123-134: The test currently checks for a WITHDRAW event using the
global goldenRuleIDHex and captures the last non-nil amount across all rows;
change it to use the test's created rule ID (the local variable that holds the
rule ID you create in this test) instead of goldenRuleIDHex and, inside the
callAs row callback (used by maa_get_events), only read and assert lastAmount
when the row corresponds to the WITHDRAW event (e.g., check row.Values[...] ==
"WITHDRAW" using the same index used for evtTypes or match the rule ID column to
the created rule ID) so the amount is taken from that specific WITHDRAW row and
the assertion becomes deterministic and rule-local.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 186fc066-0f78-4f09-afcf-0e2a09d2df11
📒 Files selected for processing (4)
internal/migrations/049-maa-funding.prod.sqlinternal/migrations/049-maa-funding.sqlscripts/generate_prod_migrations.pytests/streams/maa/withdraw_test.go
resolves: https://github.com/truflation/website/issues/4038
Summary by CodeRabbit
Release Notes