fix: assert failure on non-policy asset consolidation in CreateTransactionInternal #1277
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ctionInternal Squash of 2 commits from ElementsProject#1258 - correct application of non_policy_effective_value to policy output in KnapsackSolver - replace bad fee amount assert with error log and graceful failure in CWallet::CreateTransactionInternal (cherry picked from commit 0f92a38) Update src/wallet/coinselection.cpp Co-authored-by: Byron Hambly <byron@hambly.dev> (cherry picked from commit cf0f561)
|
@delta1 there's a test failure in |
delta1
added
the
hacktoberfest-accepted
|
Pushed the fix to the functional test for the |
|
ACK 441c751. Pushed a commit on top of it which fixes the coin selection bug that was causing the rest of the test to fail. |
Adds a functional test to cover the issue uncovered in ElementsProject#1259, where calling fundrawtransaction with many non-policy inputs and no policy recipients results in an assertion failure and a crash. Fixed in ElementsProject#1258. (cherry picked from commit a8b0ed6)
|
Thanks @jamesdorfman, testing now after fixing a bug I introduced in the test. |
|
Pushed to fix lint issues |
jamesdorfman
force-pushed
the
issues-1259
branch
from
a815cef
to
04cba29
Compare
|
ACK 04cba29 |
jamesdorfman
force-pushed
the
issues-1259
branch
7 times, most recently
from
d8cc516
to
ab8ad42
Compare
jamesdorfman
changed the title
delta1
commented
Oct 27, 2023
Comment on lines
+18
to
+22
| "-blindedaddresses=1", | ||
| "-initialfreecoins=2100000000000000", | ||
| "-con_blocksubsidy=0", | ||
| "-con_connect_genesis_outputs=1", | ||
| "-txindex=1", |
There was a problem hiding this comment.
I think these args should match the conf files from the tutorial. can be done as part of #1278
| '8f1560e209f6bcac318569a935a0b2513c54f326ee4820ccd5b8c1b1b4632373': 0, | ||
| '4fa41f2929d4bf6975a55967d9da5b650b6b9bfddeae4d7b54b04394be328f7f': 99 | ||
| } | ||
| assert self.nodes[0].gettransaction(reissuance_txid)['amount'] == expected_amt |
There was a problem hiding this comment.
I think we could use some asserts of the balances before and after this whole process. not a showstopper though.
jamesdorfman
force-pushed
the
issues-1259
branch
from
406ce9a
to
ab54314
Compare
jamesdorfman
force-pushed
the
issues-1259
branch
from
ab54314
to
e02024e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
same diff as #1258 but for
masterbranch