Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157716418)
(As above) same line or brackets here.
💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157721554)
Why not just on one line?
💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157722990)
(Same line or brackets)
💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157719967)
Could set only the first enum value for smaller diffs/less churn in future changes.
👍 TheCharlatan approved a pull request: "build: remove ancient unused define"
(https://github.com/bitcoin/bitcoin/pull/27420)
ACK 9fbc5fcd28eeefd3ad1932a2d94e5558deeb16d7
💬 jamesob commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496579012)
Big concept ACK. Will test next week (post-honeymoon).
💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157742775)
> In general, I think I'd start with the date column first, and then the warnings one.

I see that Jonas Schnelli used a date-first order as well in https://github.com/bitcoin/bitcoin/issues/3314#issuecomment-81567665.
💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1496581866)
Updated 93c56f4f203f3bf3a616863b4ec6443aa36b2f9d -> c02e451882bb1d220b944af3b444d4b529ac351a ([kernelChainName_0](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_0) -> [splitSystemArgs_1](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_0..splitSystemArgs_1)):
* Addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1157536088) for cleaning up a needl
...
💬 TheCharlatan commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1496583438)
re-ACK 54e406118926c2a526455a60f67961520dfb65da
💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1157778909)
Not sure what you mean, I think this just intends to say `verify.py` ?
💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1157782969)
Looks like this has been implemented as `pub --cleanup`. A quick test shows it working as intended. Will update the doc.
💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1496637445)
I believe I've addressed the comments here. I've chosen to add fresh comments on top rather than squashing to avoid mixing up my changes on top of @jamesob's/@achow101's.

The os/arch filters like `verify.py 22.0-osx` no longer work for me and I'm not quite sure why. Otherwise remote/local verification seem to work ok.
💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1496649369)
> Is it not possible to achieve the same with something simpler, like the following? (...)

We'd need one more `if` so that we don't stop advertising onion addresses to `127.0.0.1` peers (inbounds from tor).

But yes, I will try this alternative out.
Alternatively, I also thought of renaming `GetReachabilityFrom` to something like `GetAdvertisingScore` - since we don't use it anywhere else.

> Or why not even expand this logic to all networks and delete `CNetAddr::GetReachabilityFrom()`?
...
💬 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