Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392760668)
You're right! Done :)
achow101 closed an issue: "`feature_bind_extra.py` test fails in `test_runner` if new nodes are added"
(https://github.com/bitcoin/bitcoin/issues/33250)
🚀 achow101 merged a pull request: "test: Use extra_port() helper in feature_bind_extra.py"
(https://github.com/bitcoin/bitcoin/pull/33260)
💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392767537)
I might be wrong but I don't think there's a way to know if the peer accepted it or not.
💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2392777914)
I would rather keep the same format as `sendrawtransaction`.
💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3353787691)
Code Review ACK [`24391ed`](https://github.com/bitcoin/bitcoin/pull/32821/commits/24391ed7804a724a62034b21150c89e45ac9b625)

Just a few non-blocking nits
🤔 Prabhat1308 reviewed a pull request: "rpc: Handle -named argument parsing where '=' character is used"
(https://github.com/bitcoin/bitcoin/pull/32821#pullrequestreview-3286685012)
Code Review ACK [`24391ed`](https://github.com/bitcoin/bitcoin/pull/32821/commits/24391ed7804a724a62034b21150c89e45ac9b625)

Just left a few non-blocking nits
💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2392811188)
Same here
💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2392810900)
This feels a little wierd . can maybe change it to something like `p.methodName == rpcMethodName` or `p.rpcMethod == method`
💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2392816061)
similar change to `&(*it)` here
💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2392814895)
maybe change here to `&(*it)` ? just looks a bit cleaner.
sipa closed a pull request: "Use number of dirty cache entries in flush warnings/logs"
(https://github.com/bitcoin/bitcoin/pull/31703)
💬 sipa commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#issuecomment-3353836542)
Closing, up for grabs.
💬 fjahr commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3353839249)
re-ACK c652deb3c16b7edccb741b9b473502092c0c2638

Just addressed @ryanofsky 's comments.
💬 sipa commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353850602)
Code review ACK 656da514c5a3ee4d376b9b60f57451d3e4b6aec7
💬 sipa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353868210)
I don't know if this is really a concern in practice, because it's already compensated for by the fact that after a long time, transactions near the top of the mempool have had many chances of being mined already.

We could just get rid of expiration entirely if a time-based limit wasn't valuable, and just rely on eviction due to fullness. But if time-based expiration is valuable, then it's inevitable that a chance exists that a soon-to-be mined transaction expires.
💬 polespinasa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353874982)
> We could just get rid of expiration entirely if a time-based limit wasn't valuable

I might agree with this.

@sipa do you know the reason why this was implemented in the first place?
💬 achow101 commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353912094)
Can the same optimization be applied to `SipHashUint256` as well? Those are used by the other `Salted*Hasher`s.
💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353915093)
Yes, will do it in the next push!
💬 achow101 commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#issuecomment-3353916233)
ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
💬 davidgumberg commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353946106)
### Archeology

Time-based mempool expiry was added here: https://github.com/bitcoin/bitcoin/commit/49b6fd5663dfe081d127cd1eb11407c4d3eaf93d as part of https://github.com/bitcoin/bitcoin/pull/6722, AFAICT most of the relevant discussion happened in https://github.com/bitcoin/bitcoin/pull/6455. There are also two relevant irc discussions: [here](https://web.archive.org/web/20151016174215/http://bitcoinstats.com/irc/bitcoin-dev/logs/2015/09/28#l1443480600.0) and [here](https://bitcoin-irc.chaincod
...