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

Re-enable the test for sorted outputs #1866

Closed
cbeck88 opened this issue Apr 27, 2022 · 1 comment
Closed

Re-enable the test for sorted outputs #1866

cbeck88 opened this issue Apr 27, 2022 · 1 comment
Assignees

Comments

@cbeck88
Copy link
Contributor

cbeck88 commented Apr 27, 2022

We had to disable the test for sorted outputs, because:

  • It was never previously exercised on a block version where the sorted output requirement is active
  • It turns out the test tx only have one TxOut in it, so it is sorted for trivial reasons even when the reverse sorter is installed. This causes the test for sorting check effectiveness to pass even though the test expects it to fail.

I think we can fix this test if we go into the get_transaction function in mc_transaction_core_test_utils and make it add a second output, e.g. a change output, because then I think the test will work as intended

(Maybe worth capturing in a testing fast-follow-on ticket?

Originally posted by @sugargoat in #1827 (comment))

@jcape jcape moved this to Todo in Blockchain Core Apr 27, 2022
@jcape jcape added this to the 1.3.0 Release milestone Apr 27, 2022
@cbeck88
Copy link
Contributor Author

cbeck88 commented Apr 28, 2022

FYI the relevant test is being moved in this PR: #1875

@remoun remoun moved this from Todo to In Progress in Blockchain Core Jun 2, 2022
@remoun remoun closed this as completed Jun 2, 2022
Repository owner moved this from In Progress to Done in Blockchain Core Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants