💬 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`.
(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
(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
(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
(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`
(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
(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.
(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)
(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.
(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.
(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
(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.
(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?
(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.
(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!
(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
(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
...
(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
...
💬 achow101 commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3353970233)
ACK 451ba9ada41f687c0e4bb34d5925374a68a8f8a3
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3353970233)
ACK 451ba9ada41f687c0e4bb34d5925374a68a8f8a3
🚀 achow101 merged a pull request: "docs: Undeprecate datacarrier and datacarriersize configuration options"
(https://github.com/bitcoin/bitcoin/pull/33453)
(https://github.com/bitcoin/bitcoin/pull/33453)
💬 fanquake commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3354000703)
This will likely fix the Guix build:
```diff
--- a/contrib/guix/symbol-check.py
+++ b/contrib/guix/symbol-check.py
@@ -249,7 +249,7 @@ def check_MACHO_libraries(binary) -> bool:
return ok
def check_MACHO_min_os(binary) -> bool:
- if binary.build_version.minos == [13,0,0]:
+ if binary.build_version.minos == [14,0,0]:
return True
return False
```
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3354000703)
This will likely fix the Guix build:
```diff
--- a/contrib/guix/symbol-check.py
+++ b/contrib/guix/symbol-check.py
@@ -249,7 +249,7 @@ def check_MACHO_libraries(binary) -> bool:
return ok
def check_MACHO_min_os(binary) -> bool:
- if binary.build_version.minos == [13,0,0]:
+ if binary.build_version.minos == [14,0,0]:
return True
return False
```
💬 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-3354019309)
Thanks for the context @davidgumberg
> > How about we make the expiration time dynamic based on the position (computer by fee rate) in the mempool?
> > Txs more likely to be mined have a bigger expiry time
>
> I think this should be the opposite, the transactions that are the best candidates for time-based expiry, are the ones that are at the top of our mempool, but block after block remain unconfirmed.
Yep, you are right, makes more sense in the opposite 😅
> but I suppose the current beha
...
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3354019309)
Thanks for the context @davidgumberg
> > How about we make the expiration time dynamic based on the position (computer by fee rate) in the mempool?
> > Txs more likely to be mined have a bigger expiry time
>
> I think this should be the opposite, the transactions that are the best candidates for time-based expiry, are the ones that are at the top of our mempool, but block after block remain unconfirmed.
Yep, you are right, makes more sense in the opposite 😅
> but I suppose the current beha
...