Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 ryanofsky commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1936304636)
re: https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1926350798

Thanks ishaanam for pointing out the lines of code that answer my questions. Since original links got broken in rebase, here are stable links that point to the two lines of code which prioritize the abandoned state over the conflicted state:

- https://github.com/bitcoin/bitcoin/commit/3de89342dcd44bdde4027a91cb27793dc0231847#diff-1f2db0e4d5c12d109c7f0962333c245b49b696cb39ff432da048e9d6c08944d8L1313-R1313
- https://g
...
💬 0xB10C commented on issue "Enable sanctions enforcement in Spam Filter Code":
(https://github.com/bitcoin/bitcoin/issues/29416#issuecomment-1936306770)
> `-sanctionblocks` : Enforce sanctions by rejecting new blocks that contain sanctioned transactions

I guess you're aware that anyone using that would hard fork off of Bitcoin with such an option. A few questions for you to consider: What happens when someone is doing IBD with this option enabled? Would they hard fork at the same height as the other nodes on the network? What happens when a part of the network updates their sanctioned list while another part of the network hasn't yet updated?
...
💬 furszy commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1484606348)
> Any reason to add __func__ to this log, when it previously wasn't there?

just an old bad habit. Removed. Thanks.
🤔 furszy reviewed a pull request: "wallet: simplify and batch zap wallet txes process"
(https://github.com/bitcoin/bitcoin/pull/28987#pullrequestreview-1872874268)
Updated per feedback. Thanks Marko and achow.
💬 furszy commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1484621811)
> I'm not sure if `NONCRITICAL_ERROR` is really the right return code. It seems like something has gone critically wrong if `TxnCommit` were to fail.

The returned error code doesn't really matter. Anything different from `LOAD_OK` is an error for all the function's callers. And they behave exactly the same for all codes.
Actually, the `DBErrors` return doesn't make sense anymore after this PR changes. The function is no longer traversing the entire db nor checking for extra, unneeded, stuff
...
💬 furszy commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1484606675)
> This should explicitly call TxnAbort before returning as otherwise we will get extraenous logs about the tx being aborted before batch is destroyed.

sure. Fixed.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484624000)
Done
🤔 TheCharlatan reviewed a pull request: "RFC: build: remove confusing and inconsistent disable-asm option"
(https://github.com/bitcoin/bitcoin/pull/29407#pullrequestreview-1872906747)
Do the mentiones from the `doc/fuzzing.md` to `--disable-asm` still need to be removed? Seems like `--disable-asm` is not recognized by libsecp, it supports a package-specific asm option.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484625047)
Yes, I've added a comment for that.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484626857)
I have added a lot more comments to the test which should clarify this.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484632206)
I have added a comment before all of the tests in this file explaining how these tests create conflicts.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484632803)
Done
achow101 closed an issue: "Enable sanctions enforcement in Spam Filter Code"
(https://github.com/bitcoin/bitcoin/issues/29416)
💬 pablomartin4btc commented on pull request "doc: Update translation process guide":
(https://github.com/bitcoin/bitcoin/pull/29414#discussion_r1484665290)
I'd leave the `~/.transifexrc` in case the user creates an account in Transifex directly, instead of using another platform account like GitHub.
💬 achow101 commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#issuecomment-1936426651)
ACK 9a3c5c8697659e34d0476103af942a4615818f4e

The new code is much simpler.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#issuecomment-1936428779)
ACK 8330145ba88b39ddaa9e0bbfc2d1c198da652249
💬 achow101 commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1936432223)
ACK 13161ecf032b7a850686e5942c12222c8f3d0d52
💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1484694946)
Ah, I see. That's unfortunate as it means this optimization isn't available before some solution was found, but probably doesn't matter much.
💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1484695646)
Oh, I missed that this was just changing the expected result, not the input. Sounds good.