docs: port the ckSOL design document to the repository

[gregorydemay]

Port the ckSOL scoping and design document into docs/design.md so the design rationale lives next to the code, and link it from the README.

• Original figures are redrawn as Mermaid diagrams (architecture overview, automated and manual deposit flows, transaction submission, withdrawal, status check, and the transaction state machine).
• Content deliberately left out for a public repository: team contact details, internal tracker links, the roll-out plan, and internal working notes.
• Section numbering fixed: the original had two sections numbered 3.6; Events is now 3.5 and API is 3.6.
• Editorial corrections to the testing section: the withdrawal scenario now says no SOL is transferred, test case 2.2 links to its own sample transaction, and test case 2.3 refers to the minimum deposit amount.
• The document describes the design as written, not the current implementation. The API section points to the Candid file as the authoritative interface.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VpFpbJ5sgNutGGJUQ5dmkr

[Copilot]

🟡 Changes recommended

The new design document contains a few internal inconsistencies in the “Testing” section (withdrawal wording and sample transaction references) that should be corrected before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR ports the ckSOL scoping/design document into the repository as docs/design.md and links it from the README so the design rationale (including flows, fees, and state machine) lives alongside the code.

Changes:

• Add a new public design doc (docs/design.md) with Mermaid diagrams and design rationale.
• Link the design doc from README.md and include it in the repository tree listing.

File summaries

File	Description
README.md	Adds a link to the design doc and lists docs/design.md in the repo structure.
docs/design.md	Introduces the ported ckSOL design document with diagrams, flows, fees, and API notes.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The new design doc contains a few verified mismatches/ambiguities vs the authoritative Candid interface (notably around withdraw and the API section’s argument presentation).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

docs/design.md:591

• This endpoint list presents arguments as positional parameters, but the authoritative Candid interface uses record argument types (e.g., GetDepositAddressArgs, WithdrawalArgs). Using the Candid type names here keeps the design doc aligned with the actual public interface and avoids ambiguity about field names/order.

1. `get_deposit_address(opt principal, opt subaccount)`: Returns the Solana address derived from the provided principal ID and subaccount. If no principal ID is provided, the principal ID of the caller is used.
2. `update_balance(opt subaccount)`: Returns `ok` if the address derived from the caller's principal ID and the provided subaccount, if any, is being tracked.
3. `process_deposit(opt principal, opt subaccount, signature)`: Processes the transaction for the given signature. If the transaction is processed successfully, the deposit status is returned. Otherwise, an error is returned.
4. `withdraw(opt subaccount, amount, address)`: Burns the given amount of ckSOL from the user's account and transfers the same amount minus a fee in SOL to the given user address. Returns the block index of the burn operation on the ckSOL ledger in case of success. Otherwise, an error is returned.
5. `withdrawal_status(block_index)`: Returns the withdrawal status (`NotFound`, `Pending`, `TxSent`, `TxFinalized`) for the withdrawal identified by the given block index.

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc contains a few internal inconsistencies (notably withdraw argument order vs Candid/diagrams and some terminology/linking issues) that should be corrected to avoid misleading integrators.

Review details

Suppressed comments (9)

Previously missed (2) — in code that hasn't changed since the last review.

docs/design.md:368

• This section mixes the term “retrieval request” for what is otherwise described as a withdrawal flow; the implementation and API use “withdrawal” terminology throughout. Using a single term here will reduce confusion.

This issue also appears in the following locations of the same file:

• line 383
• line 392
  docs/design.md:525

• This sentence is phrased as a timeless claim (“There are no Solana addresses…”), but the OFAC SDN list can change over time. Rephrasing this as a check procedure avoids the doc becoming incorrect without code changes.

This issue also appears in the following locations of the same file:

• line 533
• line 560

docs/design.md:364

• The parameter order for withdraw here disagrees with both the sequence diagram below and the Candid interface (which takes from_subaccount, then amount, then address). To avoid confusing API consumers, describe the parameters in the same order as the Candid file.

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:379

• This withdraw(...) call in the sequence diagram uses (subaccount, address, amount), but the Candid interface defines WithdrawalArgs as { from_subaccount; amount; address }. The diagram should match the authoritative interface to prevent integration mistakes.

    User->>Minter: withdraw(subaccount, address, amount)

docs/design.md:387

• The diagram notes still refer to “retrieval request”; renaming to “withdrawal request” keeps terminology consistent with the rest of the doc and code.

    Note over Minter: Queue retrieval request
    Minter-->>User: burn block index

    Note over Minter: Timer fires (every 10 seconds)
    Note over Minter: Batch up to 10 retrieval requests

docs/design.md:392

• This paragraph still refers to “retrieval requests” even though the rest of the section describes withdrawals; use consistent terminology.

Since Solana has a high block rate, the timer should execute more frequently compared to ckBTC. The proposed interval is **10 seconds**. A shorter interval between calls implies that there is a lower chance of retrieval requests being batched together; however, it is preferable to have smaller batches, as transactions are cheap and it provides a better user experience.

docs/design.md:564

• This link points back to the GitHub main branch for a file that exists in this repository. Using a relative link (like the Candid link below) makes the doc work reliably in forks and in rendered docs where the repo name/branch differ.

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

docs/design.md:533

• “At the time of writing” without a date tends to age poorly; since the output is included as an example, wording it as a sample keeps the doc accurate even if the SDN list changes.

At the time of writing, running the command yields the following output.

docs/design.md:560

• This conclusion is tied to the sample output above; making that explicit keeps the statement accurate if the SDN list later adds Solana addresses.

None of the entries in the list correspond to a Solana address. Therefore, implementing any checks against the OFAC SDN list is not required. The DeFi team at DFINITY periodically checks the OFAC SDN list to update the blocklists in the ckBTC and ckETH minter. The appearance of Solana addresses can be checked at the same time.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc has a few internal inconsistencies (notably withdraw argument ordering vs the Candid interface, and diagrams/text mismatches) that should be corrected before merging.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:36

• The overview Mermaid diagram lists minter endpoints but omits update_balance, which is part of the deposit flow described later and listed in the API section; this makes the diagram inconsistent with the rest of the document.

docs/design.md:364

• This paragraph states the withdraw parameter order as (subaccount, address, amount), but the Candid interface defines WithdrawalArgs as (from_subaccount, amount, address); the doc should match the authoritative interface.

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:379

• The sequence diagram calls withdraw(subaccount, address, amount), but the Candid WithdrawalArgs order is (from_subaccount, amount, address); this mismatch can confuse API consumers.

    User->>Minter: withdraw(subaccount, address, amount)

docs/design.md:597

• This test case is about a withdrawal that fails, but the note says "no ckSOL is minted" (minting is unrelated to withdrawal). Update the wording to reflect the withdrawal failure outcome described earlier (failed Solana tx, no reimbursement).

    1. ✅ Attempts to send SOL to a Solana program always fail (no ckSOL is minted).

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The newly added design doc contains a couple of concrete documentation issues (a misleading withdrawal test note and a non-portable in-repo source link) that should be corrected before merging.

Review details

Suppressed comments (2)

docs/design.md:573

• The ckSOL event-log reference links to the canonical GitHub repo path, which can break for forks/branches and makes it harder to navigate locally. Since this design doc is now in-repo and the referenced file exists here, prefer a relative link to the local source file.

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

docs/design.md:606

• This test-case note is about withdrawals, but it says “no ckSOL is minted”, which doesn’t match the described scenario (withdrawals burn ckSOL and attempt to send SOL). This reads like a copy/paste from a mint test and is misleading.

1. Withdraw SOL to a Solana program that always fails.
    1. ✅ Attempts to send SOL to a Solana program always fail (no ckSOL is minted).

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc’s API section and one in-repo reference link should be aligned to the authoritative Candid/types and use stable relative linking to avoid misleading readers.

Review details

Suppressed comments (2)

docs/design.md:609

• The API section lists informal endpoint signatures (e.g., withdraw(opt subaccount, amount, address)) that don’t match the actual Candid interface (record args like WithdrawalArgs with from_subaccount). Since this section is meant to describe the public API, it should reflect the Candid argument types/field names to avoid confusing readers and drifting from the authoritative interface.

1. `get_deposit_address(opt principal, opt subaccount)`: Returns the Solana address derived from the provided principal ID and subaccount. If no principal ID is provided, the principal ID of the caller is used.
2. `update_balance(opt subaccount)`: Returns `ok` if the address derived from the caller's principal ID and the provided subaccount, if any, is being tracked.
3. `process_deposit(opt principal, opt subaccount, signature)`: Processes the transaction for the given signature. If the transaction is processed successfully, the deposit status is returned. Otherwise, an error is returned.
4. `withdraw(opt subaccount, amount, address)`: Burns the given amount of ckSOL from the user's account and transfers the same amount minus a fee in SOL to the given user address. Returns the block index of the burn operation on the ckSOL ledger in case of success. Otherwise, an error is returned.

docs/design.md:586

• This link points to main on GitHub, which can drift from the version of the code in this repository. Since the referenced file exists in-repo, prefer a relative link so the design doc stays accurate across branches/tags.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new documentation contains an API-parameter-order inconsistency and a time-sensitive compliance statement that should be reworded to avoid becoming incorrect.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:548

• This section states unconditionally that there are no Solana addresses in the OFAC SDN list. Since the SDN list can change over time, wording this as an absolute fact can become incorrect and undermine the rationale for skipping checks.

docs/design.md:389

• The withdrawal sequence diagram lists the withdraw parameters in a different order than the authoritative Candid interface (WithdrawalArgs is from_subaccount, amount, address). This can mislead readers following the docs.

    User->>+Minter: withdraw(subaccount, address, amount)

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design document contains a few concrete inconsistencies with the repository interface/behavior (e.g., withdraw argument order and getBlock request details) that should be corrected before approval.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:48

• The design doc says the getBlock request sets transactionDetails to null, but the codebase (e.g., integration test fixtures) uses "none". This is likely to confuse readers trying to match the implementation/RPC behavior.

docs/design.md:373

• The parameter order described for withdraw (destination address, then amount) doesn’t match the canister interface (WithdrawalArgs is from_subaccount, amount, address). Keeping the doc consistent with the Candid helps prevent integration mistakes.

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:389

• The withdrawal sequence diagram shows withdraw(subaccount, dest_addr, amount), but the documented/API order elsewhere (and the Candid args) is subaccount, amount, address. Aligning the diagram avoids ambiguity for implementers.

    User->>+Minter: withdraw(subaccount, dest_addr, amount)

docs/design.md:571

• This link points to dfinity/cksol on main, which can drift from the version of the design doc being read (and breaks in forks). Since the target is in this repo, prefer a relative link for stability.

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc has a few documentation inaccuracies/inconsistencies (API signature shapes/field names and a withdrawal test-case statement) that should be corrected before merging.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:597

• This API list describes endpoints using positional parameters (and uses principal instead of the Candid field name owner), but the authoritative interface is record-based (e.g., WithdrawalArgs, ProcessDepositArgs). Using the Candid record shapes/field names here will prevent consumer confusion.

docs/design.md:574

• The ckSOL event log link points to a hard-coded GitHub URL on the main branch; using a repo-relative link keeps the docs accurate in forks and across default-branch renames.

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

docs/design.md:607

• This test scenario is about withdrawals, but the checklist item says "no ckSOL is minted" (minting is a deposit concern). This should describe the withdrawal failure behavior (burn succeeded, Solana transfer failed, and the request ends in a failed/finalized state without reimbursement).

    1. ✅ Attempts to send SOL to a Solana program always fail (no ckSOL is minted).

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design document contains verified inconsistencies with the authoritative Candid interface (withdraw argument order) and a hard-coded GitHub link that should be made relative for correctness across forks/branches.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:389

• This sequence diagram calls withdraw(subaccount, dest_addr, amount), but the Candid interface orders the arguments as (from_subaccount, amount, address). Updating the diagram avoids client confusion.

docs/design.md:373

• The parameter order described for withdraw doesn’t match the authoritative Candid interface (minter/cksol_minter.did defines WithdrawalArgs as from_subaccount, then amount, then address). This sentence currently implies subaccount, then address, then amount, which can confuse readers implementing clients.

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:576

• This link hard-codes a GitHub URL to this repository’s event.rs. Using a relative path keeps the docs correct for forks, branches, and offline browsing.

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc has a few confirmed inconsistencies/inaccuracies (notably withdraw parameter order and withdrawal-failure wording) that should be corrected to avoid misleading readers.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:549

• "At the time of writing" is ambiguous and will age quickly; adding an explicit date helps readers assess whether this OFAC snapshot is still current.

docs/design.md:377

• In this section, the parameter order for withdraw is listed as (subaccount, address, amount), but the authoritative Candid interface defines WithdrawalArgs as { from_subaccount; amount; address }. Reordering the description here avoids confusing API consumers.

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:613

• This statement is inaccurate for a withdrawal failure scenario: withdrawals burn ckSOL first, then attempt the Solana transfer; if the transfer fails, ckSOL has already been burned (and there is currently no reimbursement flow per Section 3.2.2).

    1. ✅ Attempts to send SOL to a Solana program always fail (no ckSOL is minted).

docs/design.md:617

• The sentence says the total equals the "minimum deposit fee", but given the preceding text it appears you mean the total equals the minimum deposit amount (two transfers of half the minimum deposit amount sum back to the minimum deposit amount).

    3. ✅ [Sample transaction](https://explorer.solana.com/tx/qo9AnFCRdAPr4dZjiJ7CVpSKa3APVgGGLQ8bPVhRr4dqsCsQCvHsUnVbRad2vHTLtUvKDyxfWkoFFAxZKrtSsDE?cluster=devnet) sending half of the minimum deposit amount in two separate transfers each, resulting in a total amount of exactly the minimum deposit fee. The mint happened at block index 54.

docs/design.md:580

• This links to the repository on GitHub (dfinity/cksol), which makes the document less portable (e.g., forks, branch renames). Since the referenced file exists in this repo, a relative link will stay correct.

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc contains several terminology/API mismatches (e.g., “retrieval” vs “withdrawal”, and inconsistent withdraw parameter naming) that should be corrected for accuracy and clarity.

Review details

Suppressed comments (8)

Previously missed (3) — in code that hasn't changed since the last review.

docs/design.md:391

• The text refers to these as "retrieval" requests, but the rest of the section and the public API use "withdraw"/"withdrawal" (e.g., withdraw, withdrawal_status). This terminology mismatch makes the flow harder to follow.

This issue also appears in the following locations of the same file:

• line 407
• line 413
  docs/design.md:403

• This sequence diagram shows withdraw(subaccount, dest_addr, amount), but the public interface is record-based and commonly presented as from_subaccount, amount, address. Aligning the diagram with the interface reduces confusion for readers trying to map the design to the API.

This issue also appears on line 612 of the same file.
docs/design.md:487

• "Retrieval" here appears to be a leftover term (ckBTC uses "retrieve"), but this document otherwise uses "deposit" and "withdrawal"; using "withdrawal" keeps the fee section consistent.

docs/design.md:407

• This Mermaid step still calls it a "retrieval" request, which is inconsistent with the surrounding "withdrawal" terminology and the withdrawal_status API.

    Note over Minter: Queue retrieval request

docs/design.md:413

• This sentence uses "retrieval requests" even though the API and section are framed as withdrawals; keeping the term consistent reduces confusion.

Since Solana has a high block rate, the timer should execute more frequently compared to ckBTC. The proposed interval is **10 seconds**. A shorter interval between calls implies that there is a lower chance of retrieval requests being batched together; however, it is preferable to have smaller batches, as transactions are cheap and it provides a better user experience.

docs/design.md:387

• The withdraw parameter description doesn't match the canister interface naming (from_subaccount, amount, address) and later examples. Using the interface field names here avoids ambiguity (especially since calls are record-based, not positional).

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:612

• The API list uses a positional-style signature and an ambiguous parameter name (subaccount). The actual Candid interface uses a record with from_subaccount, amount, and address fields (see minter/cksol_minter.did). Consider reflecting those names here to match the authoritative interface.

4. `withdraw(opt subaccount, amount, address)`: Burns the given amount of ckSOL from the user's account and transfers the same amount minus a fee in SOL to the given user address. Returns the block index of the burn operation on the ckSOL ledger in case of success. Otherwise, an error is returned.

docs/design.md:623

• The testing scenario is about withdrawals, but the sub-bullet says "no ckSOL is minted". Withdrawals burn ckSOL and attempt to send SOL, so this should describe the SOL transfer outcome instead of minting.

    1. ✅ Attempts to send SOL to a Solana program always fail (no ckSOL is minted).

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new design doc has several internal inconsistencies with the authoritative Candid interface and surrounding documentation (e.g., withdraw arg order/diagram and process_deposit cycle requirements) that should be corrected before merging.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

docs/design.md:322

• The required cycles for process_deposit are configurable (process_deposit_required_cycles is an init/upgrade field and is exposed via get_minter_info), but this text states a fixed requirement of 1T cycles. This will become inaccurate if the configured value changes.

docs/design.md:387

• The withdraw parameter order here conflicts with the minter’s Candid interface (WithdrawalArgs is from_subaccount, then amount, then address). This sentence currently lists destination address before amount, which can mislead API consumers.

The `withdraw` endpoint has the following parameters: An optional subaccount, the destination address on Solana, and the amount to be withdrawn. When receiving such a request, the ckSOL minter issues an `icrc2_transfer_from` call, sending the requested amount from the account corresponding to the caller's principal ID plus the provided subaccount (if any) to its own account. Since its account is the minting account, this transfer is a burn operation, burning the given amount.

docs/design.md:404

• This sequence diagram shows withdraw(subaccount, dest_addr, amount), but the authoritative interface uses (from_subaccount, amount, address) (see minter/cksol_minter.did). The diagram should match the API to avoid confusion.

    User->>+Minter: withdraw(subaccount, dest_addr, amount)
    Minter->>+Ledger: icrc2_transfer_from(user account, cksol_minter, amount)

docs/design.md:624

• This test description refers to minting, but the scenario is about withdrawals (ckSOL → SOL). In the described design, ckSOL is burned before the Solana transaction is attempted, so the key behavior to validate is how failures are recorded/handled rather than minting.

1. Withdraw SOL to a Solana program that always fails.
    1. ✅ Attempts to send SOL to a Solana program always fail (no ckSOL is minted).
2. Mint ckSOL using multiple transfers all to the same destination account.

docs/design.md:590

• This link points to the ckSOL repo on GitHub (.../cksol/blob/main/...), which can drift from the checked-out version and makes the document less portable. Since this repository contains the referenced file, prefer a relative link (as done elsewhere in this doc for the Candid file).

The ckETH minter captures all internal state changes in [events](https://github.com/dfinity/ic/blob/master/rs/ethereum/cketh/minter/src/state/event.rs). The ckSOL minter also follows this [event-based design](https://github.com/dfinity/cksol/blob/main/minter/src/state/event.rs). Since Solana has a much shorter block time than Ethereum, the potentially faster growth of the event log must be considered. In particular, the following critical questions need to be addressed: Is there a risk that the ckSOL minter cannot be upgraded anymore because the full event log can no longer be replayed? Is there a risk that the event log will not fit into stable memory anymore?

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 8200306.

Security Overview

• 🔎 Scanned files: 2 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/58f1cdfe-c0c2-41c1-8d28-3d6efcaf3266?scanId=3ad09cf7-aa39-4ad3-8cf8-18a36308d910&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type	Relevant files
Enhancement	► docs/design.md     Add design document for ckSOL (new file)

[Copilot]

🔵 Needs a closer look

The new design doc contains inconsistent “retrieval” vs “withdrawal” terminology and an ambiguous “time of writing” reference that should be corrected for clarity.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

docs/design.md:391

• In the withdrawal flow section, the text switches from “withdrawal” to “retrieval request(s)”, which is inconsistent terminology and can confuse readers about what is being queued and batched.

This issue also appears in the following locations of the same file:

• line 407
• line 413
  docs/design.md:559

• “At the time of writing” is ambiguous in a repository doc and will become stale quickly; adding an explicit date makes the OFAC check statement easier to interpret later.

docs/design.md:407

• This diagram note uses “retrieval request”, but the surrounding section is about withdrawals; using consistent terminology (“withdrawal request”) avoids confusion.

    Note over Minter: Queue retrieval request

docs/design.md:413

• This sentence refers to “retrieval requests” even though the feature being described is withdrawals; aligning wording with the section (“withdrawal requests”) improves clarity.

Since Solana has a high block rate, the timer should execute more frequently compared to ckBTC. The proposed interval is **10 seconds**. A shorter interval between calls implies that there is a lower chance of retrieval requests being batched together; however, it is preferable to have smaller batches, as transactions are cheap and it provides a better user experience.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite