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.

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

Jump to bottom

Add Reward Input tests #551

Merged
merged 31 commits into from
Nov 30, 2023
Merged

Add Reward Input tests #551

merged 31 commits into from
Nov 30, 2023

Conversation

PhilippGackstatter
Copy link
Contributor

Add tests that use the Reward Input in various scenarios.

part of #466.

@PhilippGackstatter
Add DO with empty reward can be destroyed test
2eae7a8
@PhilippGackstatter
Export Create Signed TX func
81a3c66
@PhilippGackstatter
Add reward input pointing to disallowed output tst
bcc7faf
@PhilippGackstatter
Make transaction failure assertion more generic
6063f0b
@PhilippGackstatter
Use transaction failure listener in testsuite
bddf697
@PhilippGackstatter
Use pre-computed tx id
d4891b6
@PhilippGackstatter
Merge remote-tracking branch 'origin/develop' into reward-tests
18aa7b1
@PhilippGackstatter
Merge remote-tracking branch 'origin/develop' into reward-tests
fa05af8
@PhilippGackstatter
Fix post merge issues
d796ac9
@PhilippGackstatter
Remove unnecessary genesis commitment
3c4dcc8
@PhilippGackstatter
Simplify test setup
364df7e
@PhilippGackstatter
Fix account ID when zero
04f2ee3
@PhilippGackstatter
Finish claim validator reward implementation
4f294bc
@PhilippGackstatter
Test staking feat with 0 rewards can be removed
e90a71b
@PhilippGackstatter
Merge remote-tracking branch 'origin/develop' into reward-tests
baf447b
@PhilippGackstatter
Fix post merge issue
7d8b626
@PhilippGackstatter
Log signatures invalid
3683907
@PhilippGackstatter PhilippGackstatter mentioned this pull request Nov 28, 2023
Closed
@PhilippGackstatter PhilippGackstatter marked this pull request as ready for review November 28, 2023 13:55
cyberphysic4l
cyberphysic4l reviewed Nov 29, 2023
View reviewed changes
Copy link
Contributor

@cyberphysic4l cyberphysic4l left a comment

Choose a reason for hiding this comment

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

I would prefer to keep the createSignedTransactionWithOptions() method private and create methods for specific things we want to do. It just moves some of the complexity of the tests.

pkg/tests/reward_test.go Outdated Show resolved Hide resolved
pkg/testsuite/mock/wallet_transactions.go Outdated Show resolved Hide resolved
pkg/testsuite/mock/wallet_transactions.go Outdated Show resolved Hide resolved
@PhilippGackstatter PhilippGackstatter mentioned this pull request Nov 30, 2023
Closed
cyberphysic4l
cyberphysic4l approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@cyberphysic4l cyberphysic4l left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just my two suggestions

  • creating a w.StoredMana() method for the wallet like the potential mana one you made.
  • adding a comment linked to the issue for the TODO in rewards pointing to NFT test.

pkg/testsuite/mock/wallet_transactions.go Outdated Show resolved Hide resolved
@PhilippGackstatter PhilippGackstatter merged commit aefe705 into develop Nov 30, 2023
4 checks passed
@PhilippGackstatter PhilippGackstatter deleted the reward-tests branch November 30, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyberphysic4l cyberphysic4l cyberphysic4l approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PhilippGackstatter @cyberphysic4l