aboutsummaryrefslogtreecommitdiff
path: root/.cirrus.yml
diff options
context:
space:
mode:
authorAndrew Chow <achow101-github@achow101.com>2022-06-07 14:57:17 -0400
committerAndrew Chow <achow101-github@achow101.com>2022-06-07 15:02:53 -0400
commit79cabe3a5b03e84ae5cb78046538777fd848267b (patch)
tree89d58bff4dd451dc5db596c78fd6c1f61727a720 /.cirrus.yml
parente282764e049523439bc8adaadc002a1420122830 (diff)
parent57fb37c27599fc865f20b42a27bb9c227f384de3 (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