Skip to content

feat: value-capture fee estimation and billing with tiered pricing#506

Merged
chrisli30 merged 4 commits into
mainfrom
staging
Apr 6, 2026
Merged

feat: value-capture fee estimation and billing with tiered pricing#506
chrisli30 merged 4 commits into
mainfrom
staging

Conversation

@chrisli30
Copy link
Copy Markdown
Member

@chrisli30 chrisli30 commented Apr 6, 2026

Summary

Replaces the old monitoring/execution rate model with a three-component fee system and adds a value fee ledger for post-paid billing.

Fee Model (three components per execution)

  • Execution fee — flat per-execution platform fee in USD (default $0.02), converted to native token at execution time and collected atomically in the UserOp
  • COGS — per-node gas costs in WEI for on-chain operations (contract_write, eth_transfer, loop with on-chain runner). Estimated via conservative fallbacks in V1, collected atomically via gas reimbursement in the UserOp
  • Value fee — workflow-level percentage tier based on value-capture classification:
    • Tier 1 (0.03%): simple on-chain execution
    • Tier 2 (0.09%): improves outcome
    • Tier 3 (0.18%): failure/delay causes immediate loss
    • V1: rule-based, defaults to Tier 1 for workflows with on-chain nodes
    • Post-paid, tracked in the value fee ledger

Fee Estimation API (EstimateFees)

  • Returns all three components per-execution; no server-side totals — client computes
  • COGS gas estimation for contract_write, eth_transfer, and loop nodes (based on inner runner type)
  • Smart wallet creation detection with gas estimation via eth_estimateGas against the factory
  • Graceful fallback on RPC failure (warns and assumes wallet exists)
  • Discount rules scaffolding (planned, safe no-op when empty)

Fee Billing (execution path)

  • wrapWithReimbursement extended to include execution fee transfer alongside gas reimbursement, atomically in a single executeBatchWithValues call
  • Fee breakdown attached to each execution record (execution_fee + cogs + value_fee)
  • buildExecutionFee, buildValueFee, buildCOGSFromSteps helpers for post-execution fee assembly from real gas receipts

Value Fee Ledger (FeeLedger)

  • BadgerDB-backed ledger tracking outstanding value fees per owner
  • Individual FeeRecord audit trail per execution
  • Idempotent recording with mutex-protected read-check-write to prevent double-recording on concurrent calls
  • CheckCreditLimit for pre-execution credit gating (documented TOCTOU gap, acceptable for V1)

Loop Node Classification

  • Loop nodes classified based on inner runner type, not unconditionally as on-chain
  • On-chain runners (contract_write, eth_transfer) → gas estimated, value fee applied
  • Off-chain runners (custom_code, contract_read, rest_api, graphql) → no gas cost, no value fee

Dev Docker Workflow

  • Auto-build avaprotocol/avs-dev:latest on staging push when protobuf/avs.proto changes
  • Blacksmith runner (blacksmith-4vcpu-ubuntu-2404)
  • Concurrency group cancels in-flight builds on newer pushes
  • Auto-builds use amd64-only for fast CI; manual dispatch unchanged

Config

  • FeeRatesConfig in aggregator YAML with per-tier percentages and credit limit
  • Pointer fields (*float64) to distinguish missing from explicit zero
  • CreditLimitUSD for pre-execution gating (converted to WEI at check time)

Changed Files

  • protobuf/avs.proto / avs.pb.go — Fee, NodeCOGS, ValueFee, FeeDiscount, EstimateFees messages
  • core/taskengine/fee_estimator.go — Fee estimation with COGS, value classification, loop-aware on-chain detection
  • core/taskengine/fee_ledger.go — Value fee ledger with mutex-protected idempotent recording
  • core/taskengine/executor.go — Credit gating, fee breakdown assembly
  • core/taskengine/engine.go — Fee estimation API endpoint, fee breakdown in execution records
  • pkg/erc4337/preset/builder.go — Atomic execution fee + gas reimbursement collection
  • core/config/config.go — FeeRatesConfig with credit limit
  • .github/workflows/publish-dev-docker.yml — Auto-build on proto changes, Blacksmith runner
  • docs/FEE_ESTIMATION.md — Fee model documentation

chrisli30 and others added 2 commits April 5, 2026 17:03
Co-authored-by: Chris Li <chris@avaprotocol.org>
)

Co-authored-by: Chris Li <chris@avaprotocol.org>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 6, 2026

PR Review: Value-Capture Fee Estimation and Billing

This is a well-structured refactor replacing the old monitoring/execution rate model with a cleaner three-component fee model (execution_fee + COGS + value_fee). The tiered value-capture design and documentation are clear. Below are issues ranging from correctness bugs to minor nits.


Bugs / Correctness

1. TOCTOU race in credit gating (executor.go)

The credit check reads the outstanding balance and then lets execution proceed with no lock held between them. On a multi-goroutine executor, two tasks for the same owner can both pass the check before either records a fee. For V1 where fees are not recorded automatically this is moot in practice, but the check is technically racy and will matter once V2 recording lands. Consider per-owner advisory locking or at minimum document the known gap.

2. FeeLedger.RecordValueFee idempotency check is not atomic (fee_ledger.go)

The code does a GetKey check and then a separate BatchWrite. Two concurrent calls with the same ExecutionID can both see existingRecord == nil and both write, doubling the balance. BadgerDB supports transactions; the idempotency check and the batch write should be a single transaction.

3. buildValueFee / buildCOGSFromSteps misclassify LoopNode as always on-chain

A loop can contain only off-chain operations (custom code, contract reads). Classifying NODE_TYPE_LOOP as on-chain unconditionally inflates COGS entries and applies value-capture fees to pure off-chain loops. The same issue exists in isOnChainNode in fee_estimator.go.

4. resolveRunnerAndWalletCreation now errors on CodeAt failure (breaking behavior change)

Previously the estimator logged a warning and assumed the wallet exists (safe fallback). Now it returns an error and the entire EstimateFees call fails. If the RPC is briefly degraded, fee estimation becomes completely unavailable. The old approach was more resilient; consider restoring the fallback with a warning entry in the response.


Precision / Financial Arithmetic

5. ConvertUSDToWei uses big.NewFloat(usdAmount) floating-point precision loss

big.NewFloat inherits precision from the float64 mantissa (53 bits). For amounts like 0.02 this introduces a small but non-zero rounding error in the Wei result. The test uses a plus-or-minus 1 wei tolerance which hints the author noticed this. Consider new(big.Float).SetPrec(256) or represent fee amounts as integer micro-dollars to avoid the issue.


Dead Code

6. DiscountRules / discountRules field retained but unused

FeeEstimator.discountRules is set in both constructors but never read. EstimateFees returns an empty Discounts slice unconditionally. Either remove discountRules and getDefaultDiscountRules() or add a TODO comment to make the intent clear.

7. Duplicate godoc on wrapWithReimbursement (builder.go)

The function now has two comment blocks attached to it, the original one-liner and the new expanded block. Remove the original line.


Minor Issues

8. buildExecutionFee allocates on every call

buildExecutionFee(n.config.FeeRates) is called per execution and allocates a new FeeRates struct each time via convertFeeRatesConfig. Since FeeRatesConfig is immutable after startup, convert once at initialization and cache.

9. fee_ledger_test.go uses a custom setupTestDB instead of testutil.TestMustDB()

The rest of the codebase uses testutil.TestMustDB() for test storage. The local helper diverges from the convention and won't benefit from future improvements to the shared helper.

10. FeeRecord.Tier is a raw string (stringly-typed)

The test sets Tier: "EXECUTION_TIER_1" as a raw string. This will silently diverge from the proto enum's string representation if the enum name ever changes. Use avsproto.ExecutionTier_EXECUTION_TIER_1.String() or store the int32 value.

11. Credit-limit Wei conversion is ETH-price-dependent

CreditLimitUSD is converted to Wei via ConvertUSDToWei at check time, so a sharp drop in ETH price lowers the Wei threshold and could unexpectedly block users. This may be intentional, but deserves a comment since the behavior is non-obvious.


Positive Highlights

  • The three-component fee model with self-describing Fee{amount, unit} is clean and avoids implicit unit assumptions across API boundaries.
  • Pointer fields (*float64) in YAML parsing to distinguish "missing" from explicit zero is the right approach.
  • The FeeLedger storage key design is tidy and supports audit trails.
  • The integration tests for credit gating test the right boundary conditions.
  • Moving fee documentation to docs/FEE_ESTIMATION.md is a good call.
  • Removing the old monitoring-fee model in favour of the simpler tier model is a welcome simplification.

Chris Li added 2 commits April 6, 2026 16:26
… docker auto-build

Fee system fixes:
- Add mutex to FeeLedger.RecordValueFee to prevent double-recording on concurrent calls
- Classify loop nodes based on inner runner type (contract_write/eth_transfer = on-chain,
  custom_code/contract_read/rest_api/graphql = off-chain) instead of treating all loops as on-chain
- Restore CodeAt fallback in fee estimator — log warning and assume wallet exists on RPC failure
  instead of failing the entire EstimateFees call
- Document known TOCTOU race in executor credit gating (acceptable for V1)
- Use testutil.TestMustDB() in fee_ledger_test.go to match codebase convention
- Use proto enum .String() for FeeRecord.Tier instead of raw string literal
- Remove duplicate godoc on wrapWithReimbursement

Dev docker workflow:
- Auto-build avaprotocol/avs-dev:latest on staging push when protobuf/avs.proto changes
- Use blacksmith-4vcpu-ubuntu-2404 runner
- Add concurrency group to cancel in-flight builds
- Auto-builds use amd64-only (fast) for CI; manual dispatch unchanged
@chrisli30 chrisli30 merged commit 4e9f007 into main Apr 6, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant