Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ckbtc): Use the new KYT canister in ckbtc withdrawal flow #2240

Merged
merged 16 commits into from
Nov 13, 2024

Conversation

ninegua
Copy link
Member

@ninegua ninegua commented Oct 24, 2024

Add the new KYT canister as a dependency of the ckBTC minter, and use it instead of the old KYT canister for address checking during ckBTC -> BTC withdrawal.

Because the check_address call does not require payment, calling retrieve_btc on the ckBTC miner will no longer charge a KYT fee.

@github-actions github-actions bot added the feat label Oct 24, 2024
@ninegua ninegua force-pushed the paulliu/new-kyt-in-ckbtc-withdrawal branch 6 times, most recently from cf0acd1 to 5741961 Compare October 26, 2024 00:46
@ninegua ninegua added the CI_OVERRIDE_DIDC_CHECK Skips the backwards compatibility didc check (explain in PR description why) label Oct 26, 2024
@ninegua ninegua force-pushed the paulliu/new-kyt-in-ckbtc-withdrawal branch 10 times, most recently from a5c43b2 to 48fa8a6 Compare October 29, 2024 01:24
@ninegua ninegua changed the base branch from master to paulliu/regtest-network-kyt October 29, 2024 01:25
@ninegua ninegua force-pushed the paulliu/new-kyt-in-ckbtc-withdrawal branch from 48fa8a6 to 2742762 Compare October 29, 2024 02:00
Base automatically changed from paulliu/regtest-network-kyt to master October 31, 2024 13:18
@ninegua ninegua force-pushed the paulliu/new-kyt-in-ckbtc-withdrawal branch from 8dba269 to c0302d0 Compare November 1, 2024 07:38
@ninegua ninegua changed the base branch from master to paulliu/refactor-btc-kyt-lib November 1, 2024 07:38
Base automatically changed from paulliu/refactor-btc-kyt-lib to master November 4, 2024 06:25
@ninegua ninegua force-pushed the paulliu/new-kyt-in-ckbtc-withdrawal branch from 27b6316 to 8362c92 Compare November 4, 2024 12:21
@github-actions github-actions bot added the @idx label Nov 4, 2024
@ninegua ninegua removed the CI_OVERRIDE_DIDC_CHECK Skips the backwards compatibility didc check (explain in PR description why) label Nov 4, 2024
rs/bitcoin/ckbtc/minter/src/main.rs Outdated Show resolved Hide resolved
rs/tests/ckbtc/ckbtc_minter_kyt.rs Show resolved Hide resolved
rs/bitcoin/ckbtc/minter/src/state/audit.rs Outdated Show resolved Hide resolved
rs/bitcoin/ckbtc/minter/src/updates/retrieve_btc.rs Outdated Show resolved Hide resolved
rs/bitcoin/ckbtc/minter/src/updates/retrieve_btc.rs Outdated Show resolved Hide resolved
rs/bitcoin/ckbtc/minter/tests/tests.rs Show resolved Hide resolved
rs/tests/ckbtc/ckbtc_minter_kyt.rs Show resolved Hide resolved
@ninegua ninegua added the CI_OVERRIDE_DIDC_CHECK Skips the backwards compatibility didc check (explain in PR description why) label Nov 11, 2024
Copy link
Member

@gregorydemay gregorydemay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ninegua for the improvements! Code LGTM, one last comment is regarding RetrieveBtcOfacFailed. Maybe I'm missing something, but I have the impression we don't need it at all.

rs/bitcoin/ckbtc/minter/ckbtc_minter.did Outdated Show resolved Hide resolved
rs/bitcoin/ckbtc/minter/src/state/eventlog.rs Outdated Show resolved Hide resolved
rs/bitcoin/ckbtc/minter/src/state/eventlog.rs Outdated Show resolved Hide resolved
@ninegua ninegua added this pull request to the merge queue Nov 13, 2024
Merged via the queue into master with commit 8f7692a Nov 13, 2024
24 checks passed
@ninegua ninegua deleted the paulliu/new-kyt-in-ckbtc-withdrawal branch November 13, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI_OVERRIDE_DIDC_CHECK Skips the backwards compatibility didc check (explain in PR description why) @cross-chain-team feat @idx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants