Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 jonatack commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1157809059)
After removing these two `swap` calls in the last commit bfb9291, should we remove the unused `swap` function?

<details><summary>sample diff</summary><p>

```diff
diff --git a/src/prevector.h b/src/prevector.h
index bcab1ff00cd..2e8510acbe6 100644
--- a/src/prevector.h
+++ b/src/prevector.h
@@ -459,12 +459,6 @@ public:
return *item_ptr(size() - 1);
}

- void swap(prevector<N, T, Size, Diff>& other) noexcept
- {
- std::swap(_union, other._union);
-
...
💬 jonatack commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1496697631)
> I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is CCheckQueueSpeedPrevectorJob goes from 364.56ns down to 346.21ns.

Here are my results with `./src/bench/bench_bitcoin -filter='CCheckQueueSpeedPrevectorJob*.*'`.

master @ 369d4c03b7084de967576759545ba36a17fc18bb

```
| ns/job | job/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----
...
💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157862081)
Whoops I meant `int operator() (const OutputGroup& group1, const OutputGroup& group2) const `
⚠️ Baronz1 opened an issue: "I forgot the password of one of my wallets on Bitcoin Core App"
(https://github.com/bitcoin/bitcoin/issues/27421)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [X] I still think this issue should be opened here

### Report

There is BTC in there. What can I do?
I use a Macbook
fanquake closed an issue: "I forgot the password of one of my wallets on Bitcoin Core App"
(https://github.com/bitcoin/bitcoin/issues/27421)
💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1496755215)
Rebased ac9fee615a4f0c4d1bbed0d69486c54be4860dcb -> d31816eb5a0518c80f6cc4166fb4246acfc4decd ([`pr/ignoredconf.9`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.9) -> [`pr/ignoredconf.10`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.10), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.9-rebase..pr/ignoredconf.10)) due to conflict with #27254. Also added release note and applied review suggestions
💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157902972)
sure, pushed
💬 paten12 commented on issue "failure in wallet_basic.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/27249#issuecomment-1496777632)
> https://cirrus-ci.com/task/6282049166770176?logs=ci#L3112
>
> ```shell
> node0 2023-03-12T18:59:07.047547Z [scheduler] [wallet/wallet.h:840] [WalletLogPrintf] [default_wallet] AddToWallet 0a94aab7b4dc8ba22b88ccd6b2c9898936c7d36bf8b3e1789f6083b85f6d752a new
> test 2023-03-12T18:59:07.053000Z TestFramework (ERROR): Assertion failed
> Traceback (most recent call last):
> File "/tmp/cirrus-ci-build/ci/scratch/buil
...
💬 ajtowns commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1157916340)
(Would support changing the default to 0sat/vb though)
💬 martinus commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1496950282)
> The only exception is a slowdown at https://github.com/bitcoin/bitcoin/commit/81f67977f543faca2dcc35846f73e2917375ae79 for the DirectNontrivial benchmark, but all the other improvements look quite significant.

I think the reason why DirectNontrivial got slower when adding `noexcept` is that now the supposedly faster move operations are used when resizing the vector, but since this was implemented as a `swap`, the implementation had to swap the union which I think involves 3 copies behind th
...
💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158081088)
ah, yes. You're right, now I see why SRD will always find a solution if it exists. LGTM
💬 hebasto commented on issue "gen-manpages output depends on build options, so needs to check them":
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1497000975)
> I am not able to understand exactly what this issue is trying to resolve..I have realised that the python script talked about here generates man-pages for each binary in the src/doc/man directory..but I am not able to understand what is meant by the documentation of arguments of build parameters...can anyone please explain me and clear my confusion regarding the same.

For instance, if your system prevents `./configure` from defining `HAVE_SYSTEM`, the `bitcoind -help` output will not contai
...
💬 hebasto commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1497004515)
Concept ACK.
💬 ajtowns commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1497009569)
ACK 552684976b6df34ce563458f73812e6e494e3b0e

Looking at this, it occurs to me that we're actually being a bit wasteful scanning through ~780k historical headers 29 times (or 2.4M headers 29 times for testnet) every time we startup. Replacing `MinBIP9WarningHeight` with `MinBIP9WarningStartTime` (set it to say 2022-07-01 initially), and using it for `WarningBitsConditionChecker::BeginTime()`, and just bumping it at each release might be worthwhile.
💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158084125)
I think this should be `change_output_size` not `change_spend_size`
💬 hebasto commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1158131412)
Add a copyright header?
💬 josibake commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1497114881)
Concept ACK

> We'd also need historical snapshots of a mempool to do that. If someone has got some and is willing to test, i'd be very interested in seeing the results.

I have historical snapshots and would be happy to help run the test. Although, since running the test is not trivial, I wouldn't make a blocker for the PR.
💬 hebasto commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1158209598)
Doesn't the default `memory_order_seq_cst` memory order for `std::atomic<bool> m_request_stop` guarantee the correct behaviour?