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(cycles-minting): Cycles Minting canister refunds automatically. #3484

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daniel-wong-dfinity-org
Copy link
Contributor

@daniel-wong-dfinity-org daniel-wong-dfinity-org commented Jan 16, 2025

This happens when the memo (or icrc1_memo) in an ICP transfer is not one of the special values that CMC recognizes as signaling the purpose of the transfer.

This behavior gets triggered when one of the notify_* methods gets called.

The new behavior is not actually active yet. To turn it on, we simply need to set IS_AUTOMATIC_REFUND_ENABLED to true.

Motivation

Avoids ICP getting stuck, which can result from people making mistakes when sending ICP to the CMC.

Implementation Overview

Added issue_automatic_refund_if_memo_not_offerred. As indicated by the name, this is the heart of the implementation of this new functionality. This is called from fetch_transaction. The call is guarded behind the aforementioned IS_AUTOMATIC_REFUND_ENABLED flag.

Added MEANINGFUL_MEMOS constant. This is used to detect when automatic refund should be performed.

Minor Changes

Added AutomaticallyRefunded to NotificationStatus. This ensures that we do duplicate refunds.

Changed one parameter of fetch_transaction from AccountIdentifier to Subaccount, because you cannot go "backwards" from AccountIdentifier to Subaccount.

Factored out the memo + icrc1_memo unifying code which used to live in transaction_has_expected_memo. Now, that code lives in a new get_u64_memo function.

Future Work

Added a couple of other helpers. One is set_block_status_to_processing. This could be used to reduce code duplication in notify_* methods, but such refactoring is left to future PRs.

References

We promised to do this in this forum post.

@github-actions github-actions bot added the feat label Jan 16, 2025
@daniel-wong-dfinity-org daniel-wong-dfinity-org force-pushed the cmc-automatic-refund-daniel-wong branch 2 times, most recently from ec54c97 to 76796dc Compare January 17, 2025 13:52
…existing tests still pass, but still need new tests.
@daniel-wong-dfinity-org daniel-wong-dfinity-org force-pushed the cmc-automatic-refund-daniel-wong branch from 76796dc to 300b362 Compare January 17, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant