💬 hebasto commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1847830742)
> Looks like configure doesn't detect g++-10 as a C++20 compiler...
> I presume this will be fixed by cmake...
It [does](https://github.com/hebasto/bitcoin/pull/60) :)
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1847830742)
> Looks like configure doesn't detect g++-10 as a C++20 compiler...
> I presume this will be fixed by cmake...
It [does](https://github.com/hebasto/bitcoin/pull/60) :)
💬 fanquake commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1847839860)
Isn't it just applying the exact same workaround (internally) that we are using? There's no other way to know that GCC supports C++20, because of the bug we discussed above.
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1847839860)
Isn't it just applying the exact same workaround (internally) that we are using? There's no other way to know that GCC supports C++20, because of the bug we discussed above.
💬 hebasto commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1847850612)
> Isn't it just applying the exact same workaround (internally) that we are using? There's no other way to know that GCC supports C++20, because of the bug we discussed above.
I didn't look into CMake code for that. I only tested it, including the minimum supported CMake 3.16.
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1847850612)
> Isn't it just applying the exact same workaround (internally) that we are using? There's no other way to know that GCC supports C++20, because of the bug we discussed above.
I didn't look into CMake code for that. I only tested it, including the minimum supported CMake 3.16.
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1847884788)
After offline discussion with @sdaftuar , I've worked on a set of prospective changes and pushed them to this branch as follow-on commits.
Key change is that we will now only allow package RBF when the conflicted transactions are all in "clusters" of up to size 2. This allows us to calculate mining scores for each conflicted transaction, which means that post-cluster mempool, if we did this right, IIUC, the behavior should match with more general package RBF.
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1847884788)
After offline discussion with @sdaftuar , I've worked on a set of prospective changes and pushed them to this branch as follow-on commits.
Key change is that we will now only allow package RBF when the conflicted transactions are all in "clusters" of up to size 2. This allows us to calculate mining scores for each conflicted transaction, which means that post-cluster mempool, if we did this right, IIUC, the behavior should match with more general package RBF.
💬 achow101 commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1847943885)
Benchmark crash should be fixed.
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1847943885)
Benchmark crash should be fixed.
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1847953343)
Rebased and fixed encoding mistake I made during rebase:
```diff
diff --git a/src/node/sv2_messages.h b/src/node/sv2_messages.h
index fd6974b998..d3b257c31c 100644
--- a/src/node/sv2_messages.h
+++ b/src/node/sv2_messages.h
@@ -533,11 +533,11 @@ struct Sv2SubmitSolutionMsg
s >> m_template_id >> m_version >> m_header_timestamp >> m_header_nonce;
// Ignore the 2 byte length as the rest of the stream is assumed to be
// the m_coinbase_tx.
s.ignore(
...
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1847953343)
Rebased and fixed encoding mistake I made during rebase:
```diff
diff --git a/src/node/sv2_messages.h b/src/node/sv2_messages.h
index fd6974b998..d3b257c31c 100644
--- a/src/node/sv2_messages.h
+++ b/src/node/sv2_messages.h
@@ -533,11 +533,11 @@ struct Sv2SubmitSolutionMsg
s >> m_template_id >> m_version >> m_header_timestamp >> m_header_nonce;
// Ignore the 2 byte length as the rest of the stream is assumed to be
// the m_coinbase_tx.
s.ignore(
...
💬 sipsorcery commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1847971114)
It's something to do with `CMainSignals&`. Wherever it's used in libbitcoin_qt the msvc compiler generates a syntax error of:
`Error C2238 unexpected token(s) preceding ';'`
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1847971114)
It's something to do with `CMainSignals&`. Wherever it's used in libbitcoin_qt the msvc compiler generates a syntax error of:
`Error C2238 unexpected token(s) preceding ';'`
💬 kashifs commented on pull request "Make bitcoin-tx replaceable value optional":
(https://github.com/bitcoin/bitcoin/pull/29022#discussion_r1421163198)
Hi @pinheadmz,
Thanks for taking the time to review this. I've updated the Makefile.
For the documentation, I wanted to make it clear to the end user that if a transaction with no inputs called `tx_noinputs_replaceable` is created by running the following command:`./bitcoin-tx -create replaceable`, it should NOT be assumed that inputs later added to `tx_noinputs_replaceable` will be replaceable.
Is this already obvious? If not, is there other language that I should use to make this clear?
...
(https://github.com/bitcoin/bitcoin/pull/29022#discussion_r1421163198)
Hi @pinheadmz,
Thanks for taking the time to review this. I've updated the Makefile.
For the documentation, I wanted to make it clear to the end user that if a transaction with no inputs called `tx_noinputs_replaceable` is created by running the following command:`./bitcoin-tx -create replaceable`, it should NOT be assumed that inputs later added to `tx_noinputs_replaceable` will be replaceable.
Is this already obvious? If not, is there other language that I should use to make this clear?
...
💬 achow101 commented on issue "`-min` does not minimize wallet loading dialog":
(https://github.com/bitcoin-core/gui/issues/748#issuecomment-1848033095)
Was able replicate in 26.0, needed to use the release built binaries. I guess something in the way we build qt is causing this?
(https://github.com/bitcoin-core/gui/issues/748#issuecomment-1848033095)
Was able replicate in 26.0, needed to use the release built binaries. I guess something in the way we build qt is causing this?
💬 kevkevinpal commented on pull request "Add multiplication operator to CFeeRate":
(https://github.com/bitcoin/bitcoin/pull/29037#discussion_r1421212317)
nit: To me it confused me for a moment cause I didn't realize that f was using the operator defined on the line above, I think it would make more sense for a reader to just see the same thing above defined backwards like so
`friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(f.nSatoshisPerK * a); }`
feel free to ignore this suggestion though
(https://github.com/bitcoin/bitcoin/pull/29037#discussion_r1421212317)
nit: To me it confused me for a moment cause I didn't realize that f was using the operator defined on the line above, I think it would make more sense for a reader to just see the same thing above defined backwards like so
`friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(f.nSatoshisPerK * a); }`
feel free to ignore this suggestion though
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1848150890)
Scenario: bustabit-tiny-hot-wallet_6h-YTD-2023
| Scenario File | Current Balance | Mean #UTXO | Current #UTXO | #Deposits | #Inputs Spent | #Withdraws | #Uneconomical outputs spent | #Change Created | #Changeless | Min Change Value | Max Change Value | Mean Change Value | Std. Dev. of Change Value | Total Fees | Mean Fees per Withdraw | Cost to Empty (10 sat/vB) | Total Cost | Min Input Size | Max Input Size | Mean Input Size | Std. Dev. of Input Size | Usage |
| --- | --- | --- | --- | ---
...
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1848150890)
Scenario: bustabit-tiny-hot-wallet_6h-YTD-2023
| Scenario File | Current Balance | Mean #UTXO | Current #UTXO | #Deposits | #Inputs Spent | #Withdraws | #Uneconomical outputs spent | #Change Created | #Changeless | Min Change Value | Max Change Value | Mean Change Value | Std. Dev. of Change Value | Total Fees | Mean Fees per Withdraw | Cost to Empty (10 sat/vB) | Total Cost | Min Input Size | Max Input Size | Mean Input Size | Std. Dev. of Input Size | Usage |
| --- | --- | --- | --- | ---
...
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1848191242)
Scenario: bustabit-tiny-hot-wallet_peak-and-tail
| Scenario File | Current Balance | Mean #UTXO | Current #UTXO | #Deposits | #Inputs Spent | #Withdraws | #Uneconomical outputs spent | #Change Created | #Changeless | Min Change Value | Max Change Value | Mean Change Value | Std. Dev. of Change Value | Total Fees | Mean Fees per Withdraw | Cost to Empty (10 sat/vB) | Total Cost | Min Input Size | Max Input Size | Mean Input Size | Std. Dev. of Input Size | Usage |
| bustabit-tiny-hot-wallet_p
...
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1848191242)
Scenario: bustabit-tiny-hot-wallet_peak-and-tail
| Scenario File | Current Balance | Mean #UTXO | Current #UTXO | #Deposits | #Inputs Spent | #Withdraws | #Uneconomical outputs spent | #Change Created | #Changeless | Min Change Value | Max Change Value | Mean Change Value | Std. Dev. of Change Value | Total Fees | Mean Fees per Withdraw | Cost to Empty (10 sat/vB) | Total Cost | Min Input Size | Max Input Size | Mean Input Size | Std. Dev. of Input Size | Usage |
| bustabit-tiny-hot-wallet_p
...
📝 ajtowns opened a pull request: "202312 vbits simplify"
(https://github.com/bitcoin/bitcoin/pull/29039)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/29039)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
📝 ajtowns converted_to_draft a pull request: "versionbits refactoring"
(https://github.com/bitcoin/bitcoin/pull/29039)
Increases the encapsulation/modularity of the versionbits code, moving more of the logic into the versionbits module rather than having it scattered across validation and rpc code.
(https://github.com/bitcoin/bitcoin/pull/29039)
Increases the encapsulation/modularity of the versionbits code, moving more of the logic into the versionbits module rather than having it scattered across validation and rpc code.
💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#issuecomment-1848232374)
At the end of this sequence of patches the `VersionBitsCache` object provides a standalone interface to all the "versionbits" features needed by bitcoin core, which means that if the BIP9/speedy trial implementation is changed in future, it should be possible to do that in one place, without needing to change validation/RPC code as well.
This also simplifies and modernises some of the code, much of which has been proposed previously (eg, removing the params arguments from AbstractThresholdCon
...
(https://github.com/bitcoin/bitcoin/pull/29039#issuecomment-1848232374)
At the end of this sequence of patches the `VersionBitsCache` object provides a standalone interface to all the "versionbits" features needed by bitcoin core, which means that if the BIP9/speedy trial implementation is changed in future, it should be possible to do that in one place, without needing to change validation/RPC code as well.
This also simplifies and modernises some of the code, much of which has been proposed previously (eg, removing the params arguments from AbstractThresholdCon
...
💬 helpau commented on pull request "sync: improve CCoinsViewCache ReallocateCache":
(https://github.com/bitcoin/bitcoin/pull/28945#issuecomment-1848265773)
Hi, is this benchmark for HDD or SSD? Did you tried to run benchmark with huge dbcache and high height?
(https://github.com/bitcoin/bitcoin/pull/28945#issuecomment-1848265773)
Hi, is this benchmark for HDD or SSD? Did you tried to run benchmark with huge dbcache and high height?
💬 martinus commented on pull request "test: doc: follow-up #28368":
(https://github.com/bitcoin/bitcoin/pull/29013#discussion_r1421360505)
@maflcko I just looked through the code a bit to find other usages of `fuzzed_data_provider` that depend on the evaluation order, and there are quite a few of them. E.g.
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/addrman.cpp#L272
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/addrman.cpp#L286-L290
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/banman.cpp#L79
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/bloom_filter.cpp#L
...
(https://github.com/bitcoin/bitcoin/pull/29013#discussion_r1421360505)
@maflcko I just looked through the code a bit to find other usages of `fuzzed_data_provider` that depend on the evaluation order, and there are quite a few of them. E.g.
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/addrman.cpp#L272
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/addrman.cpp#L286-L290
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/banman.cpp#L79
* https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/bloom_filter.cpp#L
...
💬 maflcko commented on issue "Software wont launch":
(https://github.com/bitcoin/bitcoin/issues/29033#issuecomment-1848332041)
> Four hours have passed, software is stuck on "verifying blocks, press Q to close" initial screen.
The software is verifying the blocks that were downloaded previously. This may take a long time, depending on your system. You can look at the tail of the debug log to see the current exact status.
> I killed the process as it was only at 20% of "verifying", so I noticed it won't open for the entire day.
Killing means it'll likely have discarded the progress. This shouldn't cause any issu
...
(https://github.com/bitcoin/bitcoin/issues/29033#issuecomment-1848332041)
> Four hours have passed, software is stuck on "verifying blocks, press Q to close" initial screen.
The software is verifying the blocks that were downloaded previously. This may take a long time, depending on your system. You can look at the tail of the debug log to see the current exact status.
> I killed the process as it was only at 20% of "verifying", so I noticed it won't open for the entire day.
Killing means it'll likely have discarded the progress. This shouldn't cause any issu
...
💬 martinus commented on pull request "sync: improve CCoinsViewCache ReallocateCache":
(https://github.com/bitcoin/bitcoin/pull/28945#issuecomment-1848332095)
I only measured it on an SSD, and only with relatively low dbcache size so flushing is triggered more often which I think would give more accurate results. But more benchmarks are definitely appreciated :)
(https://github.com/bitcoin/bitcoin/pull/28945#issuecomment-1848332095)
I only measured it on an SSD, and only with relatively low dbcache size so flushing is triggered more often which I think would give more accurate results. But more benchmarks are definitely appreciated :)
💬 maflcko commented on issue "Software wont launch":
(https://github.com/bitcoin/bitcoin/issues/29033#issuecomment-1848332099)
You can find the debug.log in your [data dir](https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#data-directory-location).
Please be aware that the debug log might contain personally identifying information.
(https://github.com/bitcoin/bitcoin/issues/29033#issuecomment-1848332099)
You can find the debug.log in your [data dir](https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#data-directory-location).
Please be aware that the debug log might contain personally identifying information.