rpc: In utxoupdatepsbt also look for the tx in the txindex
#25939
Conversation
ishaanam
force-pushed
the
utxoupdatepsbt_with_txindex
branch
3 times, most recently
from
a1c624c to
cf9d50d
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
w0xlt
left a comment
w0xlt
left a comment
There was a problem hiding this comment.
Approach ACK.
If the txindex is enabled and updated, it should be used.
ishaanam
force-pushed
the
utxoupdatepsbt_with_txindex
branch
from
cf9d50d to
687d6c2
Compare
ishaanam
force-pushed
the
utxoupdatepsbt_with_txindex
branch
2 times, most recently
from
05f4340 to
f26d600
Compare
|
ACK f26d600d55d86099206456f394a422436d5f3ec3 |
This function is called from utxoupdatepsbt and will be modified in a following commit to allow for updating inputs with the `non_witness_utxo` as well.
ishaanam
force-pushed
the
utxoupdatepsbt_with_txindex
branch
from
f26d600 to
2ee9429
Compare
|
Rebased |
|
ACK 2ee94290bdbb66732f420dc9d74493436fb2a59f |
murchandamus
left a comment
murchandamus
left a comment
There was a problem hiding this comment.
ACK 2ee94290bdbb66732f420dc9d74493436fb2a59f
src/psbt.h
Outdated
There was a problem hiding this comment.
I see that you’re just moving this code, but could you add a little more explanation here? When I was reviewing this, I was first trying to find out what non_witness_utxos and witness_utxos were, when we needed them, and finally learned per the previous PR when we could drop them. My understanding is that we can only drop non_witness_utxos when all inputs are segwit v1.
I would suspect that we might also be able to drop witness_utxos on non-segwit inputs when we do have the corresponding non_witness_utxos, and vice versa, drop the non_witness_utxos on segwit inputs, if we do have the witness_utxos for them. Is that accurate?
| /** Removes the unnecessary non_witness_utxos from a psbt. */ | |
| /** Reduces the size of the PSBT by dropping unnecessary `non_witness_utxos` (i.e. previous transactions) from a psbt when all inputs are segwit v1. */ |
There was a problem hiding this comment.
I see that you’re just moving this code, but could you add a little more explanation here?
Done as per your suggestion above.
I would suspect that we might also be able to drop witness_utxos on non-segwit inputs when we do have the corresponding non_witness_utxos,
I don't think witness_utxos should ever get added to non-segwit inputs.
and vice versa, drop the non_witness_utxos on segwit inputs, if we do have the witness_utxos for them.
Currently I'm not sure, though in #25939 (comment) sipa said that in this situation all non_witness_utxos should be kept. Even if this is not the case, changing this behavior would be out of the scope of this PR.
…txindex Previously only the segwit utxos being spent by the psbt were looked for and added to the psbt. Now, the full transaction corresponding to each of these utxos (legacy and segwit) is looked for in the txindex and mempool and added to the psbt. If txindex is disabled and the transaction is not in the mempool, then we fall back to getting just the utxo (if segwit) from the utxo set.
ishaanam
force-pushed
the
utxoupdatepsbt_with_txindex
branch
from
2ee9429 to
6e9f8bb
Compare
|
ACK 6e9f8bb |
|
ACK 6e9f8bb |
6 participants
Previously the
utxoupdatepsbtRPC, added in #13932, only updated the inputs spending segwit outputs with thewitness_utxo, because the full transaction is needed for legacy inputs. Before this RPC looked for just the utxos in the utxo set and the mempool. This PR makes it so that if the user has txindex enabled, then the full transaction is looked for there for all inputs. If it is not found in the txindex or txindex isn't enabled, then the mempool is checked for the full transaction. If the transaction an input is spending from is still not found at that point, then the utxo set is searched and the inputs spending segwit outputs are updated with just the utxo.