Check return value of addr.GetKeyID(keyid) on custom change address change. Prior to this commit this was the only place where the return value of GetKeyID(…) was unchecked.
Clarify our assumption of addr.GetKeyID(keyid) being true in this context via an assertion.
(Note to reviewers: Let me know if this assumption is not being made. If the assumption is not made then the case of !addr.GetKeyID(keyid) should obviously be handled more gracefully than with an assert :-))
utACK 65bb605.
utACK 65bb605b71f0494d23d61f545f7638df60b23e10
This looks like it will be an instant crash if you put a P2SH address in the field.
@gmaxwell Thanks for reviewing and clarifying that the old code does not assume addr.GetKeyID(keyid) being true.
I've now changed to if (!valid || !model->havePrivKey(keyid)) { … } for the unknown change address code path - looks good? :-)
I think the code there just doesn't correctly handle p2sh keys in general. Probably needs a more extensive fixup than that, though the assert seems clearly not right. :)
Likely not neccessary if #11184 lands.
@TheBlueMatt Thanks for the notification! @dooglus PR is the better choice. Closing this PR! :-)