Skip to content

fix: Ledger create canister with --next-to transferring funds too early - #3760

Merged
fxgst merged 12 commits into
masterfrom
elias/fix-next-to
May 21, 2024
Merged

fix: Ledger create canister with --next-to transferring funds too early#3760
fxgst merged 12 commits into
masterfrom
elias/fix-next-to

Conversation

@fxgst

@fxgst fxgst commented May 15, 2024

Copy link
Copy Markdown
Contributor

Description

When creating a canister with dfx ledger create-canister --next-to on a canister that does not exist (e.g., aaaa-aa), then the funds are first transferred away from the users account, but the call then fails to create the new canister, and the funds are not returned to the user's account.

Fixes # (issue)

How Has This Been Tested?

Steps to reproduce: dfx ledger --ic create-canister --amount 0.01 --next-to aaaaa-aa $(dfx identity get-principal --ic)

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@fxgst
fxgst requested review from a team and chenyan-dfinity as code owners May 15, 2024 18:35
@adamspofford-dfinity

Copy link
Copy Markdown
Contributor

Can you add a test-case to ledger.bash?

@mraszyk

mraszyk commented May 15, 2024

Copy link
Copy Markdown
Contributor

and the funds are not returned to the user's account.

they should be if notify_create is called - is there a way to trigger that with dfx "manually"?

@adamspofford-dfinity

Copy link
Copy Markdown
Contributor

@mraszyk Yes, with dfx ledger notify create-canister.

Comment thread e2e/tests-dfx/ledger.bash
@fxgst
fxgst enabled auto-merge (squash) May 16, 2024 07:28
Comment thread e2e/tests-dfx/ledger.bash Outdated
Comment thread CHANGELOG.md Outdated
@fxgst
fxgst merged commit fb3c639 into master May 21, 2024
@fxgst
fxgst deleted the elias/fix-next-to branch May 21, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants