Repository navigation
docs(fee-abstraction): make the viem examples run and use USDC's decimals - #2384
Conversation
…mals Step 3 subtracted an 18-decimal fee from a parseEther amount and relied on step 1's functions, so it neither compiled on its own nor sent the right amount. Refs #2362.
GigaHierz
left a comment
There was a problem hiding this comment.
head 5402077 · headRefOid == fetched refs/pull/2384/head (re-fetched, equal); 0 commits behind origin/main, 1 ahead; git merge-tree --write-tree clean (tree 57015d2e); one file changed; step 1 mainnet run printed Estimated fee: 0.00334671600393 USDC; CI: broken-links, GitGuardian, StepSecurity pass (Mintlify Deployment skipping)
Verdict: REQUEST-CHANGES
Status of the #2362 boxes
- CLI USDC example uses token for
--erc20Addressand adapter for--gasCurrency, and the command is run once with output noted in the PR. The code half is done on main (build/fee-abstraction/using-fee-abstraction.mdxlines 48-57 carry token 0xcebA...118C and adapter 0x2F25...602B, both checksum-correct, from #2366). The run half is NOT done: #2366's body says "I did not run the transfers ... the 'run once with output' box is still open", and #2384 contains no CLI run, only "#2366 already fixed the CLI example, so this closes out #2362". No CLI output has been recorded in either PR. - [~] viem examples self-contained, token decimals, run against Sepolia with command and output in PR. Decimals fixed and imports shown. The PR records fee outputs and
prepareTransactionRequestserialization on Sepolia (I reproduced them), but not the commands, and the send in step 3 was not run (PR admits). "Self-contained" is not met in full, see finding 2. - [~] Every address labelled with network and decimals. Tokens are labelled. Adapters are labelled with network and role but not decimals (lines 48, 64, 105, 146, 169), and the sender/recipient addresses (108-110, 172-173) carry no network. See finding 4.
-
mint broken-links --check-redirects: "no broken links found".mint validate: passed.scripts/check-orphans.sh: "No orphan pages found." CI "Check for broken links": pass.
Closes #2362 is not justified: box 1's run is open, box 2 is partly open, box 3 is partly open. Use Refs #2362, or run the missing items and record them.
Findings
-
MEDIUM · PR body (
Closes #2362) · The PR auto-closes an issue whose definition of done has an unmet run requirement. Scenario: merge closes #2362, the "CLI command run once with output" requirement and the Sepolia send are silently dropped, and AGENTS.md section 5 ("every example that stays must run; the PR notes the command and output") is not met for the CLI block or step 3's send. Fix: change toRefs #2362until a funded Celo Sepolia run (celocli transfer:erc20 --node celo-sepoliawith Sepolia token 0x01C5C0122039549AD1493B8220cABEdD739BC44E and adapter 0xbf1441Ea57f43f35f713431001f35742c88071c7, plus step 3 sent from a throwaway funded key) is recorded in the PR with command and output, or have the author or a maintainer with a funded test key run it. Add the commands, not only outputs, to the PR body. -
MEDIUM ·
build/fee-abstraction/using-fee-abstraction.mdx:153· The stated reason the sender needs "slightly more than 1 USDC" is wrong. The sentence says the network checks the balance against gas limit times max fee per gas, implying 1 USDC plus that. In celo-org/op-geth (default branchcelo-rebase-17),core/state_transition.gobuyGas()computesbalanceCheck = GasLimit * GasFeeCapand callscanPayFee(balanceCheck)(incore/celo_state_transition.go): for a fee-currency tx it compares the fee-currency balance with that amount alone; the token-transfer amount is not part of it (onlymsg.Value, which is 0 here, is added, and only to the CELO check). Separately,subFees(mgval)debitsGasLimit * GasPrice(effective price, not the cap) from the fee-currency balance before the transfer executes. So the practical requirement is transfer amount plus the upfront fee, which is about 1 USDC when the price has not moved, and a few units more if base fee rises between estimate and inclusion. I confirmed witheth_callon Sepolia with a state override of the sender's USDC balance: withtransfer = 991086,gas = 113187,maxFeePerGas = 93.75 gweiandfeeCurrencyset, balances up to 990000 revert with "transfer amount exceeds balance" and 999999 and above succeed (eth_call skips the pre-check, so this shows the debit-then-transfer ordering, not the max-fee check). Failure scenario: a reader who holds exactly 1 USDC reads "needs slightly more because of the max-fee check", assumes 1.0017 is the exact rule, and cannot predict why a send with 1.000001 fails after a base-fee bump. Fix shape: rewrite as "The fee is taken from the sender's USDC balance before the transfer runs, so the balance must cover the transferred amount plus the fee at the time of inclusion; keep a small margin because the fee can rise after the estimate. The network also requires the balance to cover gas limit times max fee per gas, which is higher than the estimate." Keep the op-geth reference only if the sentence stays accurate. -
MEDIUM ·
build/fee-abstraction/using-fee-abstraction.mdx:91-138, 155-209· The blocks are not copy-and-run self-contained: the page never says tonpm install viem, that both blocks use top-levelawaitandimportso they must be run as ES modules (.mjsor"type": "module"), or whatPRIVATE_KEYmust look like (viem needs a0x-prefixed 32-byte hex string; an unprefixed or malformed value dies insideprivateKeyToAccountwith "invalid private key, expected hex or 32 bytes, got string"). I extracted the threejsblocks verbatim and they run only as.mjswith viem installed (2.57.2, Node 20.19.4). Failure scenario: a reader pastes intoindex.jsin a CommonJS project and getsCannot use import statement outside a moduleorawait is only valid in async functions. AGENTS.md section 5 asks for imports shown and env vars named. Fix shape: one line above step 1 ("Install viem withnpm install viemand save each script as an.mjsfile; step 3 reads the sender key fromPRIVATE_KEY, a0x-prefixed hex string"), and aCodeGroupfor the install command if wanted. -
LOW · lines 48, 64, 105-110, 146, 169-173 · Labels are incomplete against AGENTS.md section 5 and #2362 box 3. Adapters say "USDC adapter, the fee currency for USDC" with no decimals; sender and recipient lines say "Example addresses; use your own" with no network. Note the USDC adapter has no
decimals()(it reverts; verified on mainnet and Sepolia), while the USDT adapter returns 18, so the label must say what is meant. Failure scenario: a reader checks the adapter's decimals, finds a revert, and cannot tell whether the label is wrong. Fix shape:// Celo mainnet (42220), USDC adapter, fee currency (gas math in 18 decimals; no decimals() on this adapter)and// Celo mainnet (42220), example senderfor sender/recipient. Same for the two CLI comment lines. -
LOW · line 153 and lines 91-209 · Key handling and mainnet default. The step 3 prose only says "Set
PRIVATE_KEYto the sender's key." There is no warning to use a dedicated low-balance key, never hardcode or commit it, and never log it (the script itself does not print it, which is good). The CLI blocks (57, 73) still pass--privateKey [PRIVATE_KEY]on the command line, which lands in shell history andps. Both scripts target mainnet, so copying step 3 sends real USDC with no dry-run suggestion even though the PR verified on Sepolia. Fix shape: a<Warning>above step 3 (key from a secret store or env var, never committed, use a throwaway funded account, try Celo Sepolia first by switchingcelotoceloSepoliaand using the Sepolia token 0x01C5...C44E and adapter 0xbf14...71c7 from Fee currency contracts) and, for the CLI, note that--privateKeyis visible in shell history. -
LOW · lines 91-209 · USDC-only examples under a page that claims USDC, USDT and USA₮. Swapping in the USDT token and adapter works (I ran step 1 on mainnet with USDT, fee
0.002475635964296625, using a USDT holder as sender), but the output line is hardcoded... USDC, so a USDT reader prints a wrong unit, and nothing says that USDT and USA₮ adapters exposegetAdaptedToken()rather thanadaptedToken()inside the script path (line 34 does say it in prose). I did not run USA₮. If the maintainers prefer USDT as the example token, the change is the two constants, the labels and the unit string; the math is identical (6 decimals, 18-decimal adapter). Fix shape: either keep USDC and add a one-line note "for USDT or USA₮ replace both addresses with the pair from Fee currency contracts and the unit label", or switch the example to USDT. -
LOW · headings, lines 11, 26, 38, 44, 78, 82, 140, 151 · AGENTS.md section 3: a page you are editing should get sentence-case headings and no numbered headings ("Allowlisted Fee Currencies", "Adapters for Non-18-Decimal Tokens", "1. Estimate the Gas Fee", ...). The PR rewrote steps 1 and 3 but left their numbered title-case headings. Fix shape:
## Estimate the gas fee,## Prepare the transaction,## Send the transaction, etc. Slugs: sentence-casing keeps#adapters-for-non-18-decimal-tokensunchanged (inbound links infee-currencies.mdx:16,add-fee-currency.mdx:162,overview.mdx:52,56stay valid; check by hand that nothing links to#1-estimate-the-gas-feebefore dropping the numbers; I found none).
Verified good
- Head and merge state as above; diff touches only the one page.
- Step 1 on Celo mainnet (https://forno.celo.org), verbatim from the page as
.mjs: prints a plausible fee. Independent recomputation by raw JSON-RPC:eth_gasPricewith the adapter parameter = 19058747175,eth_estimateGaswithfeeCurrency= adapter = 175600, product 3.34671600393e15, which formatted in 18 decimals is exactly the printed0.00334671600393; in 6 decimals floor 3346, ceil 3347. Step 1 on Celo Sepolia with Sepolia token/adapter and a USDC holder as sender:0.00891347625(gasPrice 78750000000 times gas 113187), matching the PR. - Step 3 math on Sepolia (harness: holder as keyless sender,
sendTransactionreplaced byprepareTransactionRequest):feeInUsdc = 8914, transfer991086 = 1000000 - 8914, matching the PR. Mainnet harness: fee 3347, transfer 996653. Rounding is up (ceil of 18 to 6 decimals), so the fee is never under-withheld; the transferred amount is 1 USDC minus the fee in 6-decimal base units; no double conversion (the fee is converted once fromgasPrice * gas; the transfer amount is built withparseUnits("1", 6)); the only underflow would be a fee above 1 USDC, whichencodeFunctionDatawould reject as a negative uint256, not a realistic case. - CIP-64:
celoSepolia.serializers.transactionon the prepared request (feeCurrency set) serializes with type byte0x7b;type: "0x7b","cip64", omitted and even"eip1559"all serialize to0x7bwhenfeeCurrencyis set (so step 2'stype: "0x7b"is accepted, as the PR says). The prepared Sepolia request reportsmaxFeePerGas 93750000000againstgasPrice 78750000000; on mainnet the max fee is about 2.4 times the gas price (45458641410 vs 19058747175), so the worst-case reserve gas times maxFee (7983 units) is well above the 3347-unit fee. - Step 1's comment "estimateGas reverts if the sender holds less than 1 USDC": with a zero-balance sender the call reverts with
ERC20: transfer amount exceeds balance. hexToBigIntis now only applied to the hexeth_gasPriceresult and not toestimateGas; imports in both scripts cover everything used (encodeFunctionData,erc20Abi,formatEther,hexToBigInt,http,parseUnits,privateKeyToAccount, chain). Step 2 runs as written.- Chain roles (viem reads against both networks): mainnet USDC token 0xcebA...118C
symbol()USDC,decimals()6, not inFeeCurrencyDirectory.getCurrencies(); adapter 0x2F25...602B ingetCurrencies()(directory 0x15F344b9E6c3Cb6F0376A36A64928b13F62C6276),adaptedToken()returns the token,getAdaptedToken()reverts; USDT mainnet token 0x4806...3D5e (USD₮, 6) and adapter 0x0E2A...6f72 (18,getAdaptedToken()) correct. Sepolia (chain id 11142220) USDC token 0x01C5...C44E (6) and adapter 0xbf14...71c7 (in directory 0x9212Fb72ae65367A7c887eC4Ad9bE310BAC611BF), USD₮ pair also correct. Roles in all examples are right: token fortransferandto, adapter forfeeCurrencyandeth_gasPrice. - EIP-55:
cast to-check-sum-addressreturns the input unchanged for every address on the page that I checked (USDC token and adapter, USDT token and adapter, 0x22ae...4966, 0xDF7d...c4d7, 0xccc9...0e3D). The sender 0xccc9...0e3D held 2.244022 USDC on mainnet at the time, so step 1 runs as printed. - Private key is read from
PRIVATE_KEYand never printed or logged in either script. - AGENTS.md: root-relative links, bare
jslanguage tag,Notecomponent, no promotional wording, no emoji, no Alfajores, Celo Sepolia only. - Render:
mint dev --port 3911served/build/fee-abstraction/using-fee-abstractionwith HTTP 200 and the PR's content (feeInUsdcpresent, "slightly more than 1 USDC" present, 6 code blocks); my own Playwright tab on port 3911 showed 0 console errors.
What the PR got right
- Fixes all three viem defects named in the issue: 6-decimal math, imports shown, no
hexToBigInton the bigint. - The rounding-up conversion and
total - feedesign is arithmetically sound and reproducible; the scripts are short and self-consistent. - Honest about what was not run (the send), reports real numbers that reproduce, and keeps
PRIVATE_KEYout of the source. - Step 2
type: "0x7b"retained with an accurate justification. - It is one page, no conflicts, no stale base. The remaining work is one round of targeted edits (PR body, one sentence, one install line, labels, headings), not a rethink.
Merge-order hazards
- Single file, mergeable now; the PR touches only
build/fee-abstraction/using-fee-abstraction.mdx(I did not scan other open PRs for overlap). Any other PR editing that page (headings, CLI blocks, troubleshooting) will conflict on the same hunks, especially if the heading cleanup in finding 7 lands in a separate PR. - Merging with
Closes #2362closes the issue; if findings 1-3 are resolved as a follow-up issue, link it from the PR, otherwise the CLI run requirement vanishes.
AGENTS.md section 3 asks for sentence-case, unnumbered headings on pages being edited. No inbound link uses the numbered step anchors; the other slugs are unchanged.
…bels op-geth debits the fee before the transfer and checks gas limit times max fee separately, so the old reason for needing more than 1 USDC was wrong. Also adds setup and key-handling notes, network/decimals labels and a USDT/USA₮ note.
GigaHierz
left a comment
There was a problem hiding this comment.
head d94e360 · API headRefOid matches the fetched refs/pull/2384/head; one file changed, git merge-tree --write-tree against current main is clean; CI: Check for broken links, GitGuardian and StepSecurity pass.
Verdict: APPROVE
Thank you for the two follow-up commits. Every finding from the last round is resolved on this head, and the remaining open items are honestly disclosed in the description and tracked with Refs #2362.
Status of earlier findings
Closes #2362with unmet run boxes: fixed. The description now saysRefs #2362, lists the CLI transfer and the step 3 send as not run, and records commands as well as outputs for step 1, step 2, the read-only step 3 harness and the adapterdecimals()calls.- Wrong reason for needing "slightly more than 1 USDC": fixed. Line 162 now says the fee is taken from the sender's USDC balance before the transfer runs, so the balance must cover the amount plus the fee at inclusion, and separately that the network checks the balance against gas limit times max fee per gas. That matches
buyGas()/canPayFeein op-geth. - Scripts not copy-and-run: fixed. Line 84 covers
npm install viem,.mjs,node, and how to swap in USDT or USA₮; line 162 namesPRIVATE_KEYand says it is a0x-prefixed hex string. - Incomplete labels: fixed. Adapters carry network, role and "gas math in 18 decimals", and the USDC adapter says it has no
decimals()function. Sender and recipient lines carry the network. The two CLI comment blocks match. - Key handling and mainnet default: fixed. A
<Warning>sits above step 3 (dedicated account, env var or secret store, never commit or log, try Celo Sepolia first with the exact edit), and line 76 notes that--privateKeylands in shell history and the process list. - USDC-only output unit: fixed. The
USDCsuffix is gone from the output line, and line 84 says what to replace for USDT or USA₮. - Numbered title-case headings: fixed. All headings are sentence case and unnumbered. I grepped the repo for the old slugs (
#1-estimate-the-gas-fee,#2-prepare-the-transaction,#3-send-the-transaction): nothing links to them. The inbound anchors that do exist (#adapters-for-non-18-decimal-tokensfromadd-fee-currency.mdx,overview.mdxandfee-currencies.mdx, and#using-fee-abstraction-with-viemfromoverview.mdx) still resolve.
Verified
- The executable code in all three
jsblocks is identical to the head I ran last round: the diff since then touches only comments, the dropped unit string, prose and headings. That run reproduced the step 1 fee on mainnet and Celo Sepolia, the step 3 amounts (feeInUsdc3347 and 8914, transfertotal - fee, rounding up, no double conversion), and CIP-64 serialization (type byte0x7b). - On this head I did not re-run the scripts, because the environment's secret guard blocks running any block that handles a key. The check above rests on the unchanged code.
- Addresses on the page are EIP-55 checksummed, roles are right everywhere (token for
transferandto, adapter forfeeCurrencyandeth_gasPrice), no Alfajores, Celo Sepolia only, barejslanguage tags,Warningcomponent rather than a blockquote, frontmatter unchanged and valid.
Non-blocking notes
- The example sender in step 1 (line 114) is a third-party mainnet address that holds USDC today. The comment already says
estimateGasreverts below 1 USDC; if that address is ever emptied the example fails for readers who copy it unchanged. A note to use their own funded address would make that explicit, but it is not worth holding the PR. - The step 3
sendTransactionand thecelocli transfer:erc20example have still not been executed on-chain. Step 3's prepared request, math and serialization are verified, and the CLI block is unchanged apart from comments, so I'm comfortable merging, but #2362's "run once with output" requirement stays open and should be closed by someone with a funded Celo Sepolia key, with the command and output recorded on the issue.
Merge
Single file, no conflicts with open PRs that I could find, mergeable now. Squash-merging with the PR title as the commit.
The viem examples in the fee abstraction how-to didn't run. Step 3 treated USDC as an 18-decimal token, so it sent the wrong amount, and it used functions and imports that only existed in step 1. This rewrites steps 1 and 3 as standalone scripts that use USDC's 6 decimals, labels every address with its network and decimals as AGENTS.md section 5 asks, corrects the explanation of why the sender needs a little more than 1 USDC, and sentence-cases the headings.
Refs #2362. Two boxes stay open: nobody has run the CLI transfer, and the step 3 script has not sent a transaction on Celo Sepolia. Both need a funded key, which I don't have. #2366 fixed the CLI addresses.
Changes
erc20Abi, and no longer callshexToBigIntonestimateGas, which already returns a bigint. ThehexToBigInton the raweth_gasPriceresult stays, since that call returns hex. The fee is printed without a unit, so the line stays correct when a reader swaps in USDT.parseEther("1")and usedencodeFunctionDataandparseEtherwithout importing them. The key comes fromPRIVATE_KEY, and@celo/abisis no longer needed.buyGas()incore/state_transition.go(celo-org/op-geth,celo-rebase-17) checks the fee-currency balance against gas limit times max fee per gas, without the transfer amount (canPayFeeincore/celo_state_transition.go). It then debits gas limit times the effective gas price before the transfer runs. So the balance must cover the transfer plus the fee at inclusion, with a margin in case the base fee rises.Warningabove step 3 points out that the script sends real USDC on mainnet, covers key handling, and shows how to try it on Celo Sepolia first. A line under the CLI blocks says--privateKeyends up in shell history and the process list.npm install viem,.mjsfiles andnode, and says how to switch the scripts to USDT or USA₮.decimals()function (the call reverts on both networks), so its label says so. The USDT adapter's label says 18 decimals. Example sender and recipient addresses name the network.#1-estimate-the-gas-fee,#2-prepare-the-transactionor#3-send-the-transactionanchors. The other slugs don't change.Verification
Setup, in a scratch directory outside the repo (viem 2.55.10, Node 24.21.0):
Step 1 and step 2 on Celo mainnet, unchanged:
$ node step1.mjs Estimated fee: 0.00335641401 $ node step2.mjs # exits 0, no outputStep 1 on Celo Sepolia.
step1-sepolia.mjsis step 1 withceloSepoliaimported ascelo, the Sepolia USDC token0x01C5C0122039549AD1493B8220cABEdD739BC44Eand adapter0xbf1441Ea57f43f35f713431001f35742c88071c7, and sender0x75b9B758DcE00616969ad584894CeBc07E66e830, which held 8.897780 USDC:Step 3, read-only.
step3-harness.mjschanges three lines of the page's script: the account comes from aSENDERaddress instead ofprivateKeyToAccount(process.env.PRIVATE_KEY),sendTransactionbecomesprepareTransactionRequest, and it logs the prepared values instead of the hash:serializedTypeByteis the first byte ofchain.serializers.transaction(prepared), so both requests serialize as CIP-64.Adapter decimals:
Docs checks, with mint 4.2.746:
Not run: the CLI transfer (
celocli transfer:erc20) on either network, and the step 3 script'ssendTransaction. Nothing was signed or sent. Closing #2362 needs a funded Celo Sepolia run of both, with the command and output added here.Step 2 keeps
type: "0x7b". viem serializes"0x7b","cip64"and an omitted type the same way whenfeeCurrencyis set.