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

Open
wants to merge 12 commits 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 NOT 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.

The other feature that we promised in that thread (use icrc1_memo) was implemented in an earlier PR.

@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
@daniel-wong-dfinity-org daniel-wong-dfinity-org force-pushed the cmc-automatic-refund-daniel-wong branch from 3535075 to 66544fc Compare January 17, 2025 15:18
@daniel-wong-dfinity-org daniel-wong-dfinity-org force-pushed the cmc-automatic-refund-daniel-wong branch from f7ade83 to dc2e92c Compare January 20, 2025 17:16
@daniel-wong-dfinity-org daniel-wong-dfinity-org force-pushed the cmc-automatic-refund-daniel-wong branch from dc2e92c to 9291d8d Compare January 20, 2025 17:20
@daniel-wong-dfinity-org daniel-wong-dfinity-org force-pushed the cmc-automatic-refund-daniel-wong branch from f50d3b7 to 3be81cc Compare January 20, 2025 20:11
@daniel-wong-dfinity-org daniel-wong-dfinity-org marked this pull request as ready for review January 20, 2025 20:12
@daniel-wong-dfinity-org daniel-wong-dfinity-org requested a review from a team as a code owner January 20, 2025 20:12
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

If this pull request affects the behavior of any canister owned by
the Governance team, remember to update the corresponding
unreleased_changes.md file(s).

To acknowldge this reminder (and unblock the PR), dismiss this
code review by going to the bottom of the pull request page, and
supply one of the following reasons:

  1. Done.

  2. No canister behavior changes.

@daniel-wong-dfinity-org daniel-wong-dfinity-org dismissed github-actions[bot]’s stale review January 20, 2025 20:14

New functionality is disabled via a flag. It will be enabled once security team reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant