Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 dergoegge commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1936203492)
> might be good to recap why it was only added in BLOCK processing but not other ProcessBlocks: In every other case we already don't punish the sender of compact blocks for failing these higher level checks, while full blocks allow for punishment.

We call `ProcessBlock` when we receive a `block` message (handled in this PR) or as the result of a compact block reconstruction. Compact blocks are relayed before full validation occurs, therefore we don't punish peers for sending us invalid blocks
...
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484531064)
should cover the missing case

```suggestion
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), {tx_v2_from_v3}, *ancestors_v2_from_v3), expected_error_str);
```
🤔 instagibbs reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1872752668)
looks like if I revert TX_MAX_STANDARD_VERSION to 3, all unit/functional tests pass?

ACK 29029df5c700e6940c712028303761d91ae15847 modulo that
💬 instagibbs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1484551682)
should we be checking PoW first before doing potentially a lot of hashing?
💬 jonatack commented on issue "Enable sanctions enforcement in Spam Filter Code":
(https://github.com/bitcoin/bitcoin/issues/29416#issuecomment-1936268247)
You are free to add this feature to your own fork of this open source software, but barring unforeseen circumstances, it won't achieve consensus and be merged in this project. Like your previous issue https://github.com/bitcoin/bitcoin/issues/29137, this will probably be closed as not planned.
💬 fjahr commented on pull request "test: Fix utxo set hash serialisation signedness":
(https://github.com/bitcoin/bitcoin/pull/29399#issuecomment-1936273179)
utACK fa0ceae970242d8d6bdef150c98f04c67b06e20c
💬 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.