✅ willcl-ark closed an issue: "Bitcoin Core - Transaction without permition"
(https://github.com/bitcoin/bitcoin/issues/29049)
(https://github.com/bitcoin/bitcoin/issues/29049)
💬 willcl-ark commented on issue "Bitcoin Core - Transaction without permition":
(https://github.com/bitcoin/bitcoin/issues/29049#issuecomment-1849701776)
Hello @hugomenezes85 .
I'm sorry to hear about your unauthorised transaction. If this has really taken place without your knowledge it could indicate that someone has obtained a copy of your wallet file, private key(s) or other senstive key material in order to make this transaction. This would also correlate with Bitcoin Core not needing to open the wallet for this to happen -- Bitcoin Core did not make the transaction, only picked up that it had occured from the blockchain. I'd encourage yo
...
(https://github.com/bitcoin/bitcoin/issues/29049#issuecomment-1849701776)
Hello @hugomenezes85 .
I'm sorry to hear about your unauthorised transaction. If this has really taken place without your knowledge it could indicate that someone has obtained a copy of your wallet file, private key(s) or other senstive key material in order to make this transaction. This would also correlate with Bitcoin Core not needing to open the wallet for this to happen -- Bitcoin Core did not make the transaction, only picked up that it had occured from the blockchain. I'd encourage yo
...
🤔 vasild reviewed a pull request: "refactor: Remove pre-C++20 code, fs::path cleanup"
(https://github.com/bitcoin/bitcoin/pull/29040#pullrequestreview-1774737399)
Almost ACK 0d66eea93f1e115b2e9e00ee2e89cd967f826d22, modulo the comment below about the `u8string()` method.
(https://github.com/bitcoin/bitcoin/pull/29040#pullrequestreview-1774737399)
Almost ACK 0d66eea93f1e115b2e9e00ee2e89cd967f826d22, modulo the comment below about the `u8string()` method.
💬 vasild commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1422242907)
What about the comment that says "This method can be removed after switching to C++20"? I think indeed this method should be removed entirely.
In C++17 its prototype, according to the spec is `std::string u8string() const` which matches ours and is ok. But in C++20 it is: `std::u8string u8string() const` and it is confusing to have a mismatch. I would better drop this method like the comment says and then fix problems (if any) that arise due to that.
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1422242907)
What about the comment that says "This method can be removed after switching to C++20"? I think indeed this method should be removed entirely.
In C++17 its prototype, according to the spec is `std::string u8string() const` which matches ours and is ok. But in C++20 it is: `std::u8string u8string() const` and it is confusing to have a mismatch. I would better drop this method like the comment says and then fix problems (if any) that arise due to that.
💬 vasild commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1422226453)
nit: the commit message has comments:
```
#include <system_error> // for error_code
#include <type_traits> // for is_same
```
personally I like them and find them useful.
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1422226453)
nit: the commit message has comments:
```
#include <system_error> // for error_code
#include <type_traits> // for is_same
```
personally I like them and find them useful.
💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1849743768)
> Let me know if I should drop it and you prefer to keep it here
It is ok. I don't have a preference whether it gets merged via https://github.com/bitcoin/bitcoin/pull/29040 of via this PR. Just that it makes it to `master` ;-) Thanks!
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1849743768)
> Let me know if I should drop it and you prefer to keep it here
It is ok. I don't have a preference whether it gets merged via https://github.com/bitcoin/bitcoin/pull/29040 of via this PR. Just that it makes it to `master` ;-) Thanks!
📝 MarnixCroes opened a pull request: "doc/reduce-traffic: update/clarify max outbound connection count"
(https://github.com/bitcoin/bitcoin/pull/29052)
closes https://github.com/bitcoin/bitcoin/issues/29046
(https://github.com/bitcoin/bitcoin/pull/29052)
closes https://github.com/bitcoin/bitcoin/issues/29046
💬 willcl-ark commented on issue "Nit: Inconsistency in the docs regarding block-relay-only connections":
(https://github.com/bitcoin/bitcoin/issues/29046#issuecomment-1849754763)
The short-lived feeler connections are not counted against the full outbound count:
https://github.com/bitcoin/bitcoin/blob/09ab9d4fa731866802a6a9f9aa00d92536a8b420/src/net.cpp#L2349-L2355
...so reduce-traffic.md could be updated to mention that there are occasionally up to 11 outbound nodes, which it appears @MarnixCroes has just done :)
(https://github.com/bitcoin/bitcoin/issues/29046#issuecomment-1849754763)
The short-lived feeler connections are not counted against the full outbound count:
https://github.com/bitcoin/bitcoin/blob/09ab9d4fa731866802a6a9f9aa00d92536a8b420/src/net.cpp#L2349-L2355
...so reduce-traffic.md could be updated to mention that there are occasionally up to 11 outbound nodes, which it appears @MarnixCroes has just done :)
🚀 fanquake merged a pull request: "test: fix intermittent error in rpc_net.py (#29030)"
(https://github.com/bitcoin/bitcoin/pull/29041)
(https://github.com/bitcoin/bitcoin/pull/29041)
✅ fanquake closed an issue: "test: Intermittent issue in rpc_net.py"
(https://github.com/bitcoin/bitcoin/issues/29030)
(https://github.com/bitcoin/bitcoin/issues/29030)
🤔 josibake reviewed a pull request: "wallet: skip BnB when SFFO is enabled"
(https://github.com/bitcoin/bitcoin/pull/28994#pullrequestreview-1774837387)
ACK https://github.com/bitcoin/bitcoin/commit/1d3bc77cbe25a8492a4733841bb7d6ecd6d60d30
> Applied @murchandamus suggestion of adding inputs fee to each UTXO rather than deduct them from the target. The latter one was breaking the equivalence of the input sets.
Wasn't the test still passing before, when you were deducting them from the target? If so, I would suggest we take a closer look at this test in https://github.com/bitcoin/bitcoin/pull/28985 to ensure it's testing the thing it's suppo
...
(https://github.com/bitcoin/bitcoin/pull/28994#pullrequestreview-1774837387)
ACK https://github.com/bitcoin/bitcoin/commit/1d3bc77cbe25a8492a4733841bb7d6ecd6d60d30
> Applied @murchandamus suggestion of adding inputs fee to each UTXO rather than deduct them from the target. The latter one was breaking the equivalence of the input sets.
Wasn't the test still passing before, when you were deducting them from the target? If so, I would suggest we take a closer look at this test in https://github.com/bitcoin/bitcoin/pull/28985 to ensure it's testing the thing it's suppo
...
👍 TheCharlatan approved a pull request: "msvc: Define the same `QT_...` macros as in Autotools builds"
(https://github.com/bitcoin/bitcoin/pull/29044#pullrequestreview-1774842750)
ACK 1a5dae630df1eef9eac51557b2f1c5dba0924953
Thank you @hebasto, this fixes the build failure of #28960.
(https://github.com/bitcoin/bitcoin/pull/29044#pullrequestreview-1774842750)
ACK 1a5dae630df1eef9eac51557b2f1c5dba0924953
Thank you @hebasto, this fixes the build failure of #28960.
💬 willcl-ark commented on issue "The logo icon doesn't show properly under Wayland":
(https://github.com/bitcoin-core/gui/issues/781#issuecomment-1849838605)
I think Wayland is much stricter about loading the icon from the `.desktop` file (which must have an exactly matching name). I didn't test this any further, but running Wayland on my system with an appropriate `.desktop` file installed I do get the correct icon:

That said, I added my .desktop file manually a while ago, and I notice now that on my machine the code
...
(https://github.com/bitcoin-core/gui/issues/781#issuecomment-1849838605)
I think Wayland is much stricter about loading the icon from the `.desktop` file (which must have an exactly matching name). I didn't test this any further, but running Wayland on my system with an appropriate `.desktop` file installed I do get the correct icon:

That said, I added my .desktop file manually a while ago, and I notice now that on my machine the code
...
⚠️ kallerosenbaum opened an issue: "bumpfee doc about incrementalfee is incorrect"
(https://github.com/bitcoin/bitcoin/issues/29053)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The documentation for the `bumpfee` command states
"At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee
returned by getnetworkinfo) to enter the node's mempool."
While not strictly incorrect (the "At a minimum" part), it is very misleadi
...
(https://github.com/bitcoin/bitcoin/issues/29053)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The documentation for the `bumpfee` command states
"At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee
returned by getnetworkinfo) to enter the node's mempool."
While not strictly incorrect (the "At a minimum" part), it is very misleadi
...
🚀 fanquake merged a pull request: "msvc: Define the same `QT_...` macros as in Autotools builds"
(https://github.com/bitcoin/bitcoin/pull/29044)
(https://github.com/bitcoin/bitcoin/pull/29044)
✅ fanquake closed a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960)
(https://github.com/bitcoin/bitcoin/pull/28960)
✅ fanquake closed a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960)
(https://github.com/bitcoin/bitcoin/pull/28960)
💬 darosior commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1849887372)
This shouldn't have been closed i think @fanquake.
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1849887372)
This shouldn't have been closed i think @fanquake.
📝 fanquake reopened a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960)
By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design.
Removing `CScheduler` also allows removing the thread and exception modules from the kernel library.
To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices.
---
This PR is part of the [libbitcoinke
...
(https://github.com/bitcoin/bitcoin/pull/28960)
By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design.
Removing `CScheduler` also allows removing the thread and exception modules from the kernel library.
To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices.
---
This PR is part of the [libbitcoinke
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1849912410)
Rebased and added a simple functional test, which includes a (hopefully) deterministic tweak.
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1849912410)
Rebased and added a simple functional test, which includes a (hopefully) deterministic tweak.