feat(cketh): burn ckETH from the minter's own fee subaccount - #11065
Open
mbjorkqvist wants to merge 2 commits into
Open
feat(cketh): burn ckETH from the minter's own fee subaccount#11065mbjorkqvist wants to merge 2 commits into
mbjorkqvist wants to merge 2 commits into
Conversation
Sweeper fee funding burns ckETH out of the minter's fee subaccount
(`0x…0fee`, the ckETH ledger's fee collector), so that sweep gas never comes
out of the ETH backing ckETH 1:1.
`burn_from` cannot do this: it names the minter's *default* subaccount as the
ICRC-2 spender, and the ledger only skips the allowance check when the spender
equals the `from` account compared as a **full** account. `{minter, None}` is a
different account from `{minter, 0fee}`, so the burn is rejected with
InsufficientAllowance.
Add `burn_from_own_subaccount`, which names the same subaccount as both `from`
and the spender and therefore needs no allowance, alongside
`CKETH_FEE_SUBACCOUNT`. The existing `burn_from` keeps its behaviour and
signature; both now delegate to a shared private helper, so the error mapping
is not duplicated.
Both directions are pinned against a real ckETH ledger canister with the minter
impersonated as caller, because taking the wrong branch fails only at runtime
with a confusing InsufficientAllowance: naming the fee subaccount as spender
burns (fee account debited exactly, total supply reduced), while naming the
default subaccount is rejected and moves no funds.
`LedgerClient` is canister-side code (it reads `canister_self()`), so it is
exercised end to end once the funding task calls it; these tests pin the ledger
contract it depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds allowance-free burning from the ckETH minter’s fee subaccount.
Changes:
- Adds the canonical ckETH fee subaccount constant.
- Adds a shared burn helper supporting an owned spender subaccount.
- Tests successful fee-account burns and allowance rejection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/fee_account.rs |
Pins ledger burn and allowance behavior. |
src/lib.rs |
Defines the fee subaccount constant. |
src/ledger_client.rs |
Adds owned-subaccount burning and refactors burn logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`ledger_client.rs` carries one comment across 173 lines and none on any struct, enum or method; `lib.rs` documents none of its constants. The comments added here were out of step with both, and the rationale they carried is in the commit message and the PR description, which is where it belongs. Also drops the module header and the per-item comments in `tests/fee_account.rs` for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 22431ba. Security Overview
Detected Code Changes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of DEFI-2933 (sweeper fee funding), second of a seven-PR stack. Targets #11060.
Why
Sweep gas has to be paid for without touching the ETH that backs ckETH 1:1. It is prepaid instead: ckETH is burned from the minter's own fee subaccount — the account the ckETH ledger credits transaction fees to — before any ETH moves.
The minter cannot do that today. Its existing burn helper names the minter's default subaccount as the ICRC-2 spender, and the ledger only waives the allowance check when the spender equals the
fromaccount compared as a full account.{minter, None}is a different account from{minter, 0fee}, so burning from the fee subaccount that way is rejected withInsufficientAllowance.What
Adds a burn that names the same subaccount as both
fromand spender, so no allowance is involved, alongside the constant identifying the fee subaccount. The existing user-withdrawal burn keeps its behaviour and signature; both now share a private helper, so the error mapping is not duplicated.Both directions are pinned against a real ckETH ledger canister with the minter impersonated as caller, because taking the wrong branch fails only at runtime and only with a confusing
InsufficientAllowance: naming the fee subaccount as spender burns and debits that account exactly, while naming the default subaccount is rejected and moves nothing.LedgerClientis canister-side code — it readscanister_self()— so it is exercised end to end once the funding task calls it in PR 5. These tests pin the ledger contract it depends on in the meantime.Stack
Merge in order; each PR targets the one above it.