wallet: Prevent segfault when sending to unspendable witness#13351
Conversation
| vSolutionsRet.push_back(std::move(witnessprogram)); | ||
| return true; | ||
| } | ||
| typeRet = TX_NONSTANDARD; |
There was a problem hiding this comment.
IMO this could be in line 86 above where vSolutionsRet is also cleared/initialized. It also prevents the same problem if in the future early failure returns are added.
There was a problem hiding this comment.
Could also drop lines 154 & 155 if you move this up.
There was a problem hiding this comment.
Could also drop lines 154 & 155 if you move this up.
That would just make it more fragile. As I understand this function signature, it is meant to return a "pair" of typeRet and vSolutionsRet. Setting that pair should happen as close to the return statement as possible (regardless of early-return or not).
There was a problem hiding this comment.
IMO this could be in line 86 above where vSolutionsRet is also cleared/initialized. It also prevents the same problem if in the future early failure returns are added.
This will silence valgrind and thus hide the same problem in the future instead of preventing it.
There was a problem hiding this comment.
I could add a vSolutionsRet.clear(); in this line, if people prefer...
There was a problem hiding this comment.
I just consider best practice to reset outputs in the begin if the function has multiple exits. LGTM still.
|
utACK fa36aa7 |
|
@practicalswift The gui crashes on point and the tests I added in this commit fail valgrind. |
…able witness fa36aa7 wallet: Prevent segfault when sending to unspendable witness (MarcoFalke) Pull request description: Previously we wouldn't care about the `txnouttype`, but after 4e91820 we `switch` on the type. Tree-SHA512: 6b597aba80cb43881671ad7b3a4ad97753864e8005a05c23fdd8ee79953483c08f241b5c392a9b494298eadc5cfba895b0480d916ef4f11d122fd6196f31b84a
5 participants
Previously we wouldn't care about the
txnouttype, but after 4e91820 weswitchon the type.