Skip to content

fix(tests): claim burn into the account that owns the proof #1992

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.

Sign up for GitHub

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

Open
0xPepeSilvia wants to merge 1 commit into
base: development
Choose a base branch
from
+2 −2
Open

fix(tests): claim burn into the account that owns the proof #1992

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integration_tests/tests/features/transfer.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Feature: Account transfers
Then indexer INDEXER has scanned to at least height 40

When I wait for proof BURN_PROOF to confirm on wallet MINOTARI_WALLET
When I claim burn BURN_PROOF and spend it into account ACCOUNT using wallet daemon WALLET_D
When I claim burn BURN_PROOF and spend it into account ACC using wallet daemon WALLET_D
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 10, 2026

Choose a reason for hiding this comment

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

medium

While this change fixes the ownership proof validation error by claiming the burn into the default account (ACC), it creates an inconsistency with the test's stated intent. The comments at lines 21 and 30 indicate that the burn is intended to fund the 'sender account' (ACCOUNT). By claiming the funds into ACC instead, the sender account does not receive the burned tokens, and the test no longer verifies that these funds are correctly credited to and usable by the sender. A more robust fix would be to set ACCOUNT as the default account before the burn step (using 'When I set the default account for WALLET_D to ACCOUNT') or to update the burn step definition to accept a specific account name.


# Wait for the wallet daemon account monitor to update the sender account information

Expand Down Expand Up @@ -80,7 +80,7 @@ Feature: Account transfers
Then indexer INDEXER has scanned to at least height 40

When I wait for proof BURN_PROOF to confirm on wallet MINOTARI_WALLET
When I claim burn BURN_PROOF and spend it into account ACCOUNT_1 using wallet daemon WALLET_D
When I claim burn BURN_PROOF and spend it into account ACC using wallet daemon WALLET_D
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 10, 2026

Choose a reason for hiding this comment

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

medium

Similar to the change at line 28, claiming the burn into ACC instead of ACCOUNT_1 fixes the validation failure but bypasses the intended funding of the sender account. The comment at line 85 still refers to funding the 'sender account', which is now inaccurate since the funds are going to ACC. Consider setting ACCOUNT_1 as the default account before the burn to maintain the test's logic and coverage.


# Fund the sender account with faucet tokens
When I print the cucumber world
Expand Down
⚡