diff options
author | Andrew Chow <achow101-github@achow101.com> | 2022-06-07 14:57:17 -0400 |
---|---|---|
committer | Andrew Chow <achow101-github@achow101.com> | 2022-06-07 15:02:53 -0400 |
commit | 79cabe3a5b03e84ae5cb78046538777fd848267b (patch) | |
tree | 89d58bff4dd451dc5db596c78fd6c1f61727a720 /.cirrus.yml | |
parent | e282764e049523439bc8adaadc002a1420122830 (diff) | |
parent | 57fb37c27599fc865f20b42a27bb9c227f384de3 (diff) |
Merge bitcoin/bitcoin#25239: wallet: 'CommitTransaction', remove extra wtx lookup and add exception for db write error
57fb37c27599fc865f20b42a27bb9c227f384de3 wallet: CommitTransaction, remove extra wtx lookup and add exception for a possible db write error. (furszy)
Pull request description:
Two points for `CWallet::CommitTransaction`:
1) The extra wtx lookup:
As we are calling to `AddToWallet` first, which returns the recently added/updated wtx pointer, there is no need to look up the wtx again few lines later. We can just use it.
2) The db write error:
`AddToWallet` can only return a nullptr if the db write fails, which inside `CommitTransaction` translates to an exception throw cause. We expect everywhere that `CommitTransaction` always succeed.
------------------------------------------------
Extra note:
This finding generated another working path for me :)
It starts with the following question: why are we returning a nullptr from `AddToWallet` if the db write failed without removing the recently added transaction from the wallet's map?..
Can led to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map.
-- I'm writing it here to gather some feedback first and not forget it, will create a follow-up PR in the coming days 🚜 --
ACKs for top commit:
achow101:
ACK 57fb37c27599fc865f20b42a27bb9c227f384de3
jonatack:
ACK 57fb37c
ryanofsky:
Code review ACK 57fb37c27599fc865f20b42a27bb9c227f384de3. Seems like a clear improvement. Better to fail earlier with a better error message if the failure is going to happen anyway
Tree-SHA512: 80e59c01852cfbbc70a5de1a1c2c59b5e572f9eaa08c2175112cb515256e63fa04c7942f92a513b620d6b06e66392029ebe8902287c456efdbee58a7a5ae42da
Diffstat (limited to '.cirrus.yml')
0 files changed, 0 insertions, 0 deletions