You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
docs: name the transaction submission flow and correct the stale sections - #251
Audits the documentation against the code and fixes everything that had drifted from it.
The section this branch was opened for is the largest piece. It was titled "Consolidation" and did two jobs: it documented the consolidation mechanism, which the removal of the signature-based deposit flow deleted, and it documented the generic flow that submits a Solana transaction carrying a recent block hash, which sweeps still follow. It now documents only the second and is named after it, and no mention of consolidation is left in the document.
Two sections are removed outright:
The walk-through of a getTransaction response for a user's deposit transfer. The ckSOL minter never inspects those transactions: it reads the balance of the deposit address and credits the deposit from the metadata of its own sweep, so the only transactions it parses are the ones it created and signed. The remaining subsections of 3.1 are renumbered accordingly, so the transaction submission flow is now Section 3.1.3.
The testing section. Test scenarios and their results do not belong in a design document, and two of its three scenarios credited a deposit from the shape of a user's transfer, which the balance sweep replaced.
The rest were claims the code no longer supports:
The frequency of the sweep timer and the number of deposits one round covers, and likewise the interval of the withdrawal processing timer. Whether to close the gap to the originally proposed interval is marked as still open.
Batching several transactions into a single HTTPS outcall, stated as fact although it was never implemented. It is now marked as a deviation that is still open.
The signature and fee analysis, which read as if it were generic but only ever applied to sweeps. It is scoped to them and contrasted with the single signature of a withdrawal.
The commitment levels a sweep uses for its block hash, for its preflight simulation and for its expiry check, which this stack changed and which were documented nowhere.
The minContextSlot of the balance check, which is deliberately not set.
The response sizes every RPC call is priced with. The minter sets no estimate on any request, so all of them use the SOL RPC canister's defaults, which the document described per method instead. The per-call costs and every total derived from them follow: the recent block attempt, the happy path and worst case of a single-deposit sweep, the withdrawal transaction, and the worked example of the deposit fee. Both fees keep their margin, and the costs the minter absorbs rather than charging are now named.
The cost derivation of the automatic deposit fee, dropped together with the flow it sized, which has to derive it again when it is revisited.
The API list, which offered an endpoint that does not exist and omitted one that does.
The ledger suite, described with archive canisters that the disabled archive trigger threshold never spawns.
The owners deposit_sol rejects, which include the minter's own principal and not only the anonymous one.
The README additionally points at the deployment guide, which nothing linked to.
Rebased onto main. The branch predated the durable-nonce stack, so every conflict resolved in favour of main and the section rename was reapplied on current content.
…ions
Section 3.1.4 was titled "Consolidation" and did two jobs: it documented
the consolidation mechanism, which the removal of the signature-based
deposit flow deleted, and it documented the generic flow that submits a
Solana transaction carrying a recent block hash, which sweeps still
follow. It now documents only the second and is named after it, the
cross-references are repointed, and no mention of consolidation is left.
The sections that had drifted from the code are corrected along with it:
- The sweep timer runs every minute, not every ten, and a round covers
batches of ten deposits up to a hundred rather than a single batch.
- Batching several transactions into one HTTPS outcall is marked as an
unimplemented deviation instead of being stated as fact.
- The signature and fee analysis is scoped to sweeps, whose signers are
the deposit addresses, and contrasted with the single signature of a
withdrawal transaction.
- The block hash of a sweep is read at the confirmed commitment level,
with the preflight simulation at the same level and the expiry check
still at finalized, which was previously undocumented.
- The balance check sets no minContextSlot, and the reasons why it does
not need to replace the claim that it does.
- The getTransaction and getSignatureStatuses response sizes and cycle
costs follow the SOL RPC canister defaults, and the withdrawal fee is
derived from the corrected figures.
- The cost derivation of the automatic deposit fee is dropped: the
parameter it sized does not exist, and the automated flow has to
derive its fee again when it is revisited.
- The API list no longer offers update_balance, which does not exist,
and lists get_events.
- Section 3.1.1 is marked outdated, since the minter only ever parses
transactions it created and signed itself.
The README now points at the deployment guide, which nothing linked to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gregorydemay
changed the title
docs: name the generic transaction submission flow after what it does
docs: name the transaction submission flow and correct the stale sections
Oct 9, 2026
The section walked through a `getTransaction` response for a user's
deposit transfer. The ckSOL minter never inspects those transactions: it
reads the balance of the deposit address and credits the deposit from the
metadata of its own sweep, so the only transactions it parses are the
ones it created and signed. The remaining subsections of 3.1 are
renumbered and the references to them follow.
The worst-case RPC cost that the deposit_sol fee must cover drops to
12.0B cycles with the corrected `getTransaction` estimate, leaving the
45B fee a wider margin over the 38.2B it has to pay for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 12.0B figure is what a single-deposit sweep costs when every RPC call
succeeds on its first attempt, not its worst case. Fetching a recent
block is configured for up to three attempts of one getSlot and one
getBlock each, so it can cost 12.9B rather than 4.3B cycles, which brings
such a sweep to about 46.8B cycles and past the 45B fee.
The derivation now says which case the fee covers and groups the block
fetch retries with the two costs the minter already absorbed, the
getTransaction retries and the batched status checks of the finalization
timer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clarify supported deposit flow versus deferred automated design
docs/design.md:84
This still says two deposit flows are supported, but Section 3.1.1 immediately states that the automated flow is not implemented, and the Candid service exposes no update_balance endpoint. Please distinguish the deferred design from the single supported manual flow.
Clarify fee coverage for additional signature status checks
docs/design.md:574
This conflicts with the preceding derivation, which explicitly includes one getSignatureStatuses call in the 12.0B happy-path cost covered by the fee. Only subsequent status checks are absorbed by the minter; qualify this as “additional” checks so the fee accounting is internally consistent.
► README.md The diff updates deployment and design references in docs/design.md and docs/deployment.md, and modifies wording in various design sections to reflect updated flow terminology.
Enhancement
► docs/design.md Rename and reorganize sections: Automated Flow, Manual Flow, Transaction Submission, and Sweep/Finalization references updated to reflect new section numbering and flow descriptions ► docs/deployment.md Add deployment guide reference and cross-linking to design changes in design.md
The ckSOL minter sets no response size estimate on any request, so every
call is priced with the SOL RPC canister's default for that method, not
only getTransaction and getSignatureStatuses. The list of sizes that the
costs were derived from is replaced by that statement, and a paragraph
reconciles the smaller default for getTransaction with the much larger
response measured over arbitrary mainnet transactions: the minter only
ever fetches the transactions it built itself, which are capped at the
maximum transaction size, and an underestimate is retried with double the
estimate anyway.
The per-call costs of getAccountInfo, getBalance, getSlot and getBlock
follow, and with them every total derived from those figures: the recent
block attempt, the happy path and worst case of a single-deposit sweep,
the withdrawal transaction, and the worked example of the deposit fee.
No conclusion changes; both fees keep their margin.
Also corrected in the same pass:
- The withdrawal processing timer runs every minute with a 10 second
retry, rather than the 10 second interval originally proposed, and the
gap is marked as still open.
- A diagram note pointed at section 3.2.2 for the finalization timer,
which is now the nonce account setup.
- The transaction submission flow is introduced for sweeps rather than
for all transactions, since withdrawals carry a durable nonce value.
- The ledger suite is described without archive canisters, which the
disabled archive trigger threshold never spawns.
- deposit_sol rejects the minter's own principal as the deposit owner,
not only the anonymous principal.
- The last test scenario of the testing section, which had no content,
is dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test scenarios and their results do not belong in a design document. The
section also no longer described the minter: two of its three scenarios
credited a deposit from the shape of a user's transfer, which the balance
sweep replaced, and the sample transactions under them were recorded
against that flow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The overview of the deposit flows said that two of them are supported,
and the manual flow derived its deposit address "as in the automated
flow", while the automated flow is deferred to a post-launch upgrade and
carries a warning saying so. The overview now names which of the two is
implemented, and the manual flow no longer compares itself to a flow that
does not exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The configured threshold is 4,200,000,000 blocks (docs/deployment.md:171-175), which makes archiving effectively disabled but does not place it beyond the ledger's nat64 block range or guarantee that archives can never spawn. Please avoid the absolute claim and describe the operational intent instead.
Distinguish implemented and proposed deposit flows
docs/design.md:83
This says two deposit flows are supported, but the warning immediately below states that the automated flow is not implemented, and the API list correctly omits update_balance. Distinguish the implemented manual flow from the proposed automated flow so this overview does not advertise an unavailable feature.
Correct the explanation of deposit address derivation
docs/design.md:242
The rejection is documented correctly, but its explanation is not: every account address, including one owned by the minter principal, uses the non-empty schema-v1 derivation path (minter/src/address/mod.rs:139-145), while the main address uses the empty path (minter/src/address/mod.rs:19-22). Remove this causal clause rather than identifying that derived deposit address as the main address.
Clarify included versus additional status checks
docs/design.md:566
The preceding 12.3B happy-path total explicitly includes one getSignatureStatuses call, but this sentence says the finalization timer's status checks are outside the fee. Qualify this as additional checks after the first one included in the estimate so the fee derivation is internally consistent.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Two statements remain inaccurate regarding deposit-address derivation and RPC cost inputs.
0 open findings
Previously missed (2)
In code that hasn't changed since last review
Correct inaccurate rationale for rejecting the minter principal
docs/design.md:242
The rationale for rejecting the minter principal is incorrect: its account still derives a schema-v1 deposit address, and address/tests.rs:78-82 explicitly verifies that this address differs from the main address. This also contradicts the invariant documented above on line 83 that every deposit address uses a non-empty path. Remove this false explanation (or document the actual ledger-level reason for the rejection).
Clarify getTransaction cost uses the 10 KiB default estimate
docs/design.md:551
This says the estimates use the measured response sizes listed above, but the new getTransaction cost of 3.2B is based on the SOL RPC canister's 10 KiB default, not the listed 31,985-byte measurement. That contradicts lines 547-549 and makes the cost derivation ambiguous; identify the default estimates as the inputs instead.
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
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.
mbjorkqvist
left a comment
Summary
Audits the documentation against the code and fixes everything that had drifted from it.
The section this branch was opened for is the largest piece. It was titled "Consolidation" and did two jobs: it documented the consolidation mechanism, which the removal of the signature-based deposit flow deleted, and it documented the generic flow that submits a Solana transaction carrying a recent block hash, which sweeps still follow. It now documents only the second and is named after it, and no mention of consolidation is left in the document.
Two sections are removed outright:
getTransactionresponse for a user's deposit transfer. The ckSOL minter never inspects those transactions: it reads the balance of the deposit address and credits the deposit from the metadata of its own sweep, so the only transactions it parses are the ones it created and signed. The remaining subsections of 3.1 are renumbered accordingly, so the transaction submission flow is now Section 3.1.3.The rest were claims the code no longer supports:
minContextSlotof the balance check, which is deliberately not set.deposit_solrejects, which include the minter's own principal and not only the anonymous one.The README additionally points at the deployment guide, which nothing linked to.
Rebased onto
main. The branch predated the durable-nonce stack, so every conflict resolved in favour ofmainand the section rename was reapplied on current content.Documentation only, no code changes.
🤖 Generated with Claude Code