Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ“ darosior opened a pull request: "script: return verification flag responsible for error upon validation failure"
(https://github.com/bitcoin/bitcoin/pull/33012)
For unconfirmed transactions, we currently distinguish between Script validation failures due to standardness rules and those due to consensus rules. The distinction is used to decide whether to disconnect the peer which submitted this transaction.

This is currently achieved by repeating Script validation with only the consensus verification flags, after it failed with the standardness flags. It is wasteful, and potentially significantly more so since we have Taproot. This PR proposes to repl
...
πŸ€” glozow reviewed a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-3034612261)
light code review ACK 96da68a38fa, looks correct to me
πŸ’¬ glozow commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2216772534)
When I changed this to `false`, none of the tests failed - maybe worth adding a multisig case?
πŸ’¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2216808190)
This line is counting ops in the scriptSig. I'm not sure it makes sense to add a test case with an `OP_CHECKMULTISIG` sneaked in the scriptSig because this is already non-standard.
πŸ’¬ achow101 commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3090534874)
light ACK 50024620b909fc30b68a3715680e963f048482a5
πŸš€ achow101 merged a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829)
πŸ’¬ achow101 commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3090586828)
ACK 96da68a38fa295d2414685739c41b8626e198d27
πŸš€ achow101 merged a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521)
πŸ’¬ caesrcd commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3090651380)
> Also, i don't think there is a good reason for decreasing the incremental relay fee at the same time as the min relay fee.

If `DEFAULT_INCREMENTAL_RELAY_FEE` is not lowered along with `DEFAULT_MIN_RELAY_TX_FEE`, users will be forced to pay fees above the estimates during periods of low transaction demand. It’s as if `DEFAULT_INCREMENTAL_RELAY_FEE` were now effectively set to 10,000 sat/kvB.

That’s exactly what I’m seeing. Yesterday I broadcast a transaction with a 0.2 sat/vB fee, and wit
...
πŸ’¬ naiyoma commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325)
I was testing https://github.com/bitcoin/bitcoin/pull/32928 and made an attempt to reproduce this issue by hanging a getdescriptors call

```git diff
index aa9d286417..edbd5359ee 100755
--- a/test/functional/mock_external_signer.py
+++ b/test/functional/mock_external_signer.py
@@ -97,6 +97,15 @@ parser_signtx.add_argument('psbt', metavar='psbt')
parser_signtx.set_defaults(func=signtx)

if not sys.stdin.isatty():
+ log.debug("About to read from stdin")
+ log.debug(f"Current args: {sys.a
...
πŸ’¬ marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2216903066)
Done. Thanks for the suggestion, this is quite a bit simpler. Let me know if there's something that can be improved/fixed.
πŸ“ darosior opened a pull request: "Backport #32521 to 29"
(https://github.com/bitcoin/bitcoin/pull/33013)
This backports PR #32521 to make the change available to miners who can't (or don't want to) upgrade past version 29.
πŸ’¬ tnndbtc commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090725560)
@naiyoma I also suspect this might be the cause and put comment in https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3071393448 . As stated in that comment, one possible reason is that the close of `err_wr_pipe`, which should send the empty string to stdin for the python process (signer.py) to confirm, could return `-1` but bitcoind didn't check the return result.
`subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below`

Typically, close of file descr
...
πŸ’¬ fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216970656)
I agree, sounds good to me!
πŸ‘ ryanofsky approved a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-3033783450)
Code review ACK 248b6a27c351690d3596711cc36b8102977adeab. Looks good! Thanks for adapting this and considering all the suggestions. I did leave more comments below but non are important and this looks good as-is
πŸ’¬ ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216425882)
In commit "scripted-diff: unify xor-vs-obfuscation nomenclature" (0b8bec8aa6260c499c2663ab7a1c905da0d312c3)

Given latest changes where the obfuscation object is what's serialized instead of the key (e.g. `Read(OBFUSCATION_KEY_KEY, m_obfuscation)`), I think it probably makes sense to rename OBFUSCATE_KEY_KEY to OBFUSCATION_KEY
πŸ’¬ ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216849672)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211613705

> I think the current way is probably simpler. Please let me know if you disagree.

I think current way is ok, but serialization test in the diff has some benefits over the current one. Diff suggested:

```c++
BOOST_AUTO_TEST_CASE(obfuscation_serialize)
{
Obfuscation obfuscation{};

// Test loading a key.
std::vector<std::byte> key_in{m_rng.randbytes<std::byte>(Obfuscation::KEY_SIZE)};
DataStr
...
πŸ’¬ ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216948127)
In commit "optimization: peel align-head and unroll body to 64 bytes" (248b6a27c351690d3596711cc36b8102977adeab)

reinterpret_cast seems more appropriate than bit_cast here, though either are probably ok. IIUC reinterpret cast is guaranteed to convert the pointer to an integer that can be round tripped back into a valid pointer, while bit_cast is looser and doesn't provide any guarantees.
πŸ’¬ ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216935443)
re: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2210851772

In commit "optimization: peel align-head and unroll body to 64 bytes" (248b6a27c351690d3596711cc36b8102977adeab)

I was about to ask the same question about why min was there to handle an impossible condition and would suggest either removing the `min` or removing the `if (target.size() > KEY_SIZE)` check if the latter wouldn't hurt performance . Either of these would make the code easier to follow, IMO
πŸ’¬ ryanofsky commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2216962117)
In commit "optimization: peel align-head and unroll body to 64 bytes" (248b6a27c351690d3596711cc36b8102977adeab)

Might be clearer to s/64-byte/8*KEY_SIZE/ here and s/64-bit/KEY_SIZE/ below