💬 1440000bytes commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1478277908)
I am not against sipa as I understand fee estimation is difficult however `estimatesmartfee` sucks and could be improved.
There are lot of occasions when I found it to be irrelevant and just look at mempool.space or multiple explorers
One of the project that I found more useful: https://github.com/TrueLevelSA/btc-congestion-manager
Anyway, this PR tries to improve how projects use mempool info and should not have been controversial at any point.
Request to all developers including
...
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1478277908)
I am not against sipa as I understand fee estimation is difficult however `estimatesmartfee` sucks and could be improved.
There are lot of occasions when I found it to be irrelevant and just look at mempool.space or multiple explorers
One of the project that I found more useful: https://github.com/TrueLevelSA/btc-congestion-manager
Anyway, this PR tries to improve how projects use mempool info and should not have been controversial at any point.
Request to all developers including
...
💬 ryanofsky commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1478301187)
> I did not address anything directly related to `bitcoin-cli`, I wasn't able to reproduce any issues and I'm not exactly sure what your cli issue was. Can you provide steps and unexpected behavior for that?
The issue with bitcoind described https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043 is that a config file specifying a datadir which contains a second config file will ignore the second config file with no warning. I outlined three solutions for this issue here: http
...
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1478301187)
> I did not address anything directly related to `bitcoin-cli`, I wasn't able to reproduce any issues and I'm not exactly sure what your cli issue was. Can you provide steps and unexpected behavior for that?
The issue with bitcoind described https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043 is that a config file specifying a datadir which contains a second config file will ignore the second config file with no warning. I outlined three solutions for this issue here: http
...
💬 furszy commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143775844)
Thanks ishaanam.
At least for me, it doesn't look right to check the vector size while we loop over it. It is also not correct to use `outputs.size()` as `outputs` will be empty if the user didn't customize them.
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143775844)
Thanks ishaanam.
At least for me, it doesn't look right to check the vector size while we loop over it. It is also not correct to use `outputs.size()` as `outputs` will be empty if the user didn't customize them.
👍 brunoerg approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
re-ACK 60c3f4918190900e5f79341abcc0878214656257
(https://github.com/bitcoin/bitcoin/pull/27257)
re-ACK 60c3f4918190900e5f79341abcc0878214656257
💬 TheBlueMatt commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1478325196)
Can you (here or in a separate PR) add support for validating release artifacts without actually downloading them (ie pre-downloaded release artifacts)?
(https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1478325196)
Can you (here or in a separate PR) add support for validating release artifacts without actually downloading them (ie pre-downloaded release artifacts)?
💬 achow101 commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478331026)
ACK 2c3a90f663a61ee147d785167a2902494d81d34d
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478331026)
ACK 2c3a90f663a61ee147d785167a2902494d81d34d
✅ hebasto closed a pull request: "Accurately account for mempool index memory "
(https://github.com/bitcoin/bitcoin/pull/26614)
(https://github.com/bitcoin/bitcoin/pull/26614)
🚀 achow101 merged a pull request: "Log new headers"
(https://github.com/bitcoin/bitcoin/pull/27278)
(https://github.com/bitcoin/bitcoin/pull/27278)
💬 mzumsande commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478339894)
> Unconditionally logging headers (outside of IBD) makes sense to me. It doubles the number of messages per block, but since we check their proof-of-work, it's not too bad.
It triples them, the unconditional log output when we receive a header via compact block looks like this with this PR:
```
2023-03-21T17:10:55Z Saw new header hash=0000000000000000000198c04342dff96f7940111d874c7bc73730a33b268225 height=781829
2023-03-21T17:10:55Z [net] Saw new cmpctblock header hash=000000000000000000
...
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478339894)
> Unconditionally logging headers (outside of IBD) makes sense to me. It doubles the number of messages per block, but since we check their proof-of-work, it's not too bad.
It triples them, the unconditional log output when we receive a header via compact block looks like this with this PR:
```
2023-03-21T17:10:55Z Saw new header hash=0000000000000000000198c04342dff96f7940111d874c7bc73730a33b268225 height=781829
2023-03-21T17:10:55Z [net] Saw new cmpctblock header hash=000000000000000000
...
✅ hebasto closed a pull request: "test: Make `mempool_tests/MempoolSizeLimitTest` allocation-neutral"
(https://github.com/bitcoin/bitcoin/pull/26615)
(https://github.com/bitcoin/bitcoin/pull/26615)
💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478368582)
@mzumsande this can be reduced to one, but requires some additional complexity: https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142467276
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478368582)
@mzumsande this can be reduced to one, but requires some additional complexity: https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142467276
💬 furszy commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143819895)
oops, fixed. And added coverage for it.
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143819895)
oops, fixed. And added coverage for it.
💬 ishaanam commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143830313)
> It is also not correct to use outputs.size() as outputs will be empty if the user didn't customize them.
In what scenario would we need to make the change output the sole recipient if the user didn't customize the outputs? My understanding was that this is only an issue because of the addition of the "outputs" vector.
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143830313)
> It is also not correct to use outputs.size() as outputs will be empty if the user didn't customize them.
In what scenario would we need to make the change output the sole recipient if the user didn't customize the outputs? My understanding was that this is only an issue because of the addition of the "outputs" vector.
💬 furszy commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143834924)
a send-to-self, single change output tx that is bumped.
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143834924)
a send-to-self, single change output tx that is bumped.
💬 jonatack commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1143851851)
@josibake Our clang-format wouldn't agree. It would revert your diff to what was merged (and propose this change instead).
```diff
@@ -4224,7 +4224,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (received_new_header) {
LogPrintfCategory(BCLog::NET, "Saw new cmpctblock header hash=%s peer=%d\n",
- blockhash.ToString(), pfrom.GetId());
+ blockhash.ToString(), pfrom.GetId());
...
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1143851851)
@josibake Our clang-format wouldn't agree. It would revert your diff to what was merged (and propose this change instead).
```diff
@@ -4224,7 +4224,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (received_new_header) {
LogPrintfCategory(BCLog::NET, "Saw new cmpctblock header hash=%s peer=%d\n",
- blockhash.ToString(), pfrom.GetId());
+ blockhash.ToString(), pfrom.GetId());
...
💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143839532)
I could be reading this code wrong, but isn't each tx taking inputs from each tx before it? It's still parents-and-child technically but it's also a chain.
```suggestion
// Where each tx takes an input from all prior txns
```
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143839532)
I could be reading this code wrong, but isn't each tx taking inputs from each tx before it? It's still parents-and-child technically but it's also a chain.
```suggestion
// Where each tx takes an input from all prior txns
```
💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143826570)
Why this size exactly - the first tx isn't guaranteed to be the smallest or anything? Why not just 0?
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143826570)
Why this size exactly - the first tx isn't guaranteed to be the smallest or anything? Why not just 0?
💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143849591)
Why? I don't think it has any impact on the construction performance?
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143849591)
Why? I don't think it has any impact on the construction performance?
💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143851697)
nit: I think we tend to prefer this
```suggestion
for (size_t i{0}; i < txns.size(); ++i) {
```
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143851697)
nit: I think we tend to prefer this
```suggestion
for (size_t i{0}; i < txns.size(); ++i) {
```
💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143841355)
Er, what does this comment mean?
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143841355)
Er, what does this comment mean?
💬 jonatack commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#issuecomment-1478433249)
Sorry for the drive-by suggestion: from the title, I thought this PR might be about the peer eviction benchmarks. Maybe update the title to s/eviction/mempool eviction/.
(https://github.com/bitcoin/bitcoin/pull/27019#issuecomment-1478433249)
Sorry for the drive-by suggestion: from the title, I thought this PR might be about the peer eviction benchmarks. Maybe update the title to s/eviction/mempool eviction/.