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

fix: refactor incorrect cucumber test on burn funds via cli #4679

Conversation

jorgeantonio21
Copy link
Contributor

Description

Refactor previously incorrect cucumber for burn funds via cli test.

Motivation and Context

PR #4655 introduced a specific cucumber for testing burn funds via cli. This test was incorrectly designed. This PR addresses the situation.

How Has This Been Tested?

Cucumber

@jorgeantonio21 jorgeantonio21 marked this pull request as ready for review September 16, 2022 12:32
stringhandler
stringhandler previously approved these changes Sep 16, 2022
Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

Please see the comments below. The exact amounts used in the comments are not that important, but the principle is. I think the desired behaviour should be that we can create a burn transaction that utilizes all UTXOs in the wallet expecting a change amount just smaller than the smallest UTXO.

Then I have mining node MINER_B mines 10 blocks
Then I get balance of wallet WALLET_A is at least 10000000000 uT via command line
When I wait for wallet WALLET to have at least 221552530060 uT
When I create a burn transaction of 201552500000 uT from WALLET via command line
Copy link
Contributor

Choose a reason for hiding this comment

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

This line currently does not spend all UTXOs in the wallet, but it shoud.

The average coinbase for the 1st 12 blocks is 18 462 710 838 uT. This amount should be large enough to utilize all available UTXOs in the transaction with expected change less than one coinbase worth, for example
221 552 530 060 uT - 205 089 819 222 uT = 16 462 710 838 uT. This will result in an empty wallet until the change UTXO is received.

Suggested change
When I create a burn transaction of 201552500000 uT from WALLET via command line
When I create a burn transaction of 205089819222 uT from WALLET via command line
And I restart wallet WALLET
Then wallet WALLET detects all transactions are at least Broadcast
Then I wait for wallet WALLET to have less than 1 uT

We can also restart the wallet here as waiting for the change UTXO to be received and confirmed does not need to be via the command line

When I create a burn transaction of 201552500000 uT from WALLET via command line
When I mine 5 blocks on BASE
Then all nodes are at height 20
# Then I wait for wallet WALLET to have at least 100 uT
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Then I wait for wallet WALLET to have at least 100 uT
Then I wait for wallet WALLET to have at least 100 uT

This line is important to ensure the wallet received the change UTXO (worth 16 462 710 838 uT in this example)

When I mine 5 blocks on BASE
Then all nodes are at height 20
# Then I wait for wallet WALLET to have at least 100 uT
Then I get balance of wallet WALLET is at most 18462621580 uT via command line
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Then I get balance of wallet WALLET is at most 18462621580 uT via command line
Then I get balance of wallet WALLET is at most 16462710838 uT via command line

And mining node MINER_A mines 15 blocks
And I have wallet WALLET connected to base node BASE
And I have mining node MINER connected to base node BASE and wallet WALLET
And mining node MINER mines 12 blocks
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could probably mine fewer blocks to make the calculations easier

@stringhandler stringhandler merged commit cd183ef into tari-project:development Sep 20, 2022
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.

4 participants