⚠️ matidau opened an issue: "The following table ifc"
(https://github.com/bitcoin/bitcoin/issues/29271)

(https://github.com/bitcoin/bitcoin/issues/29271)

✅ maflcko closed an issue: "The following table ifc"
(https://github.com/bitcoin/bitcoin/issues/29271)
(https://github.com/bitcoin/bitcoin/issues/29271)
📝 remyers opened a pull request: "wallet: fix coin selection tracing to return -1 when no change pos"
(https://github.com/bitcoin/bitcoin/pull/29272)
This is a bugfix for from when [optional was introduced](https://github.com/bitcoin/bitcoin/pull/25273/commits/758501b71391136c33b525b1a0109b990d4f463e) for `change_pos` in the wallet. When optional `change_pos` is unset, we should return -1 and not 0.
I added two new checks to the `test/functional/interface_usdt_coinselection.py` which adds coverage for the situations when `normal_create_tx_internal` and `aps_create_tx_internal` events occur with no change.
You can reproduce this bug u
...
(https://github.com/bitcoin/bitcoin/pull/29272)
This is a bugfix for from when [optional was introduced](https://github.com/bitcoin/bitcoin/pull/25273/commits/758501b71391136c33b525b1a0109b990d4f463e) for `change_pos` in the wallet. When optional `change_pos` is unset, we should return -1 and not 0.
I added two new checks to the `test/functional/interface_usdt_coinselection.py` which adds coverage for the situations when `normal_create_tx_internal` and `aps_create_tx_internal` events occur with no change.
You can reproduce this bug u
...
💬 remyers commented on pull request "wallet: fix coin selection tracing to return -1 when no change pos":
(https://github.com/bitcoin/bitcoin/pull/29272#issuecomment-1897978185)
You might want to take a look at this @murchandamus. I discovered the problem while trying to repo your coingrinder results with the coin-selection-simulation scripts.
(https://github.com/bitcoin/bitcoin/pull/29272#issuecomment-1897978185)
You might want to take a look at this @murchandamus. I discovered the problem while trying to repo your coingrinder results with the coin-selection-simulation scripts.
👍 TheCharlatan approved a pull request: "build: always set `-g -O2` in `CORE_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/29205#pullrequestreview-1829103258)
lgtm ACK 00c1e2aa4496b5f038ae5199dbd16d8313766533
(https://github.com/bitcoin/bitcoin/pull/29205#pullrequestreview-1829103258)
lgtm ACK 00c1e2aa4496b5f038ae5199dbd16d8313766533
⚠️ ri131211 opened an issue: "Hello"
(https://github.com/bitcoin/bitcoin/issues/29273)
(https://github.com/bitcoin/bitcoin/issues/29273)
✅ fanquake closed an issue: "Hello"
(https://github.com/bitcoin/bitcoin/issues/29273)
(https://github.com/bitcoin/bitcoin/issues/29273)
:lock: fanquake locked an issue: "Hello"
(https://github.com/bitcoin/bitcoin/issues/29273)
(https://github.com/bitcoin/bitcoin/issues/29273)
💬 0xB10C commented on pull request "wallet: fix coin selection tracing to return -1 when no change pos":
(https://github.com/bitcoin/bitcoin/pull/29272#discussion_r1457097947)
nit: I think this tracepoint would be easier to reason about if it was one line per argument. Not sure if this PR is the right place to reformat it.
(https://github.com/bitcoin/bitcoin/pull/29272#discussion_r1457097947)
nit: I think this tracepoint would be easier to reason about if it was one line per argument. Not sure if this PR is the right place to reformat it.
💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-1898030061)
`20a9fc83bd...cd5bbb12e0`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-1898030061)
`20a9fc83bd...cd5bbb12e0`: rebase due to conflicts
💬 0xB10C commented on pull request "wallet: fix coin selection tracing to return -1 when no change pos":
(https://github.com/bitcoin/bitcoin/pull/29272#discussion_r1457111240)
```
(mocktwallet/spend.cpp:1339:5: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
#0 0x5589c92846e6 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1339:5
#1 0x5589c8d8617d in wallet::spend_tests::wallet_duplicated_preset_
...
(https://github.com/bitcoin/bitcoin/pull/29272#discussion_r1457111240)
```
(mocktwallet/spend.cpp:1339:5: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
#0 0x5589c92846e6 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1339:5
#1 0x5589c8d8617d in wallet::spend_tests::wallet_duplicated_preset_
...
💬 0xB10C commented on issue "Timeout downloading block":
(https://github.com/bitcoin/bitcoin/issues/12291#issuecomment-1898052245)
Is this fixed with https://github.com/bitcoin/bitcoin/pull/27626?
cc @instagibbs
(https://github.com/bitcoin/bitcoin/issues/12291#issuecomment-1898052245)
Is this fixed with https://github.com/bitcoin/bitcoin/pull/27626?
cc @instagibbs
💬 ArmchairCryptologist commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1898054824)
As far as I can tell, the default value for `datacarriersize` has always been named `MAX_OP_RETURN_RELAY`, so claiming that it was intended to be applicable to anything else seems like a bit of stretch.
It's not that people are disagreeing that storing cat pictures on the blockchain is a bad idea, they disagree with the proposed way to limit it, as it would be a game of whack-a-mole. If you wanted to specifically target people using the segwit discount to store data cheaply in general, a bet
...
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1898054824)
As far as I can tell, the default value for `datacarriersize` has always been named `MAX_OP_RETURN_RELAY`, so claiming that it was intended to be applicable to anything else seems like a bit of stretch.
It's not that people are disagreeing that storing cat pictures on the blockchain is a bad idea, they disagree with the proposed way to limit it, as it would be a game of whack-a-mole. If you wanted to specifically target people using the segwit discount to store data cheaply in general, a bet
...
💬 ArmchairCryptologist commented on issue "Timeout downloading block":
(https://github.com/bitcoin/bitcoin/issues/12291#issuecomment-1898097298)
Looks good to me. Grepping the debug logs, I'm not seeing any block download timeouts on my public nodes with ~500 incoming connections each since updating to 26.0 early December, except for once on one node, which occurred shortly after startup and before the mempool had finished loading. It looks like it didn't have a lot of peers when the block arrived, so that's probably not a real concern.
(https://github.com/bitcoin/bitcoin/issues/12291#issuecomment-1898097298)
Looks good to me. Grepping the debug logs, I'm not seeing any block download timeouts on my public nodes with ~500 incoming connections each since updating to 26.0 early December, except for once on one node, which occurred shortly after startup and before the mempool had finished loading. It looks like it didn't have a lot of peers when the block arrived, so that's probably not a real concern.
💬 fanquake commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1898118752)
Looks like the improvement is a bit more pronouced for x86_64, at least when using Clang (17.0.6). For example, comparing `MurmurHash3`, master (left), and this PR (right), we get less code and more `rol` instructions: https://www.diffchecker.com/I7s5JeTr/. For GCC 13.2, this PR makes no difference to the generated assembly: https://www.diffchecker.com/nM5DgKfN/.
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1898118752)
Looks like the improvement is a bit more pronouced for x86_64, at least when using Clang (17.0.6). For example, comparing `MurmurHash3`, master (left), and this PR (right), we get less code and more `rol` instructions: https://www.diffchecker.com/I7s5JeTr/. For GCC 13.2, this PR makes no difference to the generated assembly: https://www.diffchecker.com/nM5DgKfN/.
🚀 fanquake merged a pull request: "refactor: C++20: Use std::rotl"
(https://github.com/bitcoin/bitcoin/pull/29085)
(https://github.com/bitcoin/bitcoin/pull/29085)
💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1898145002)
Rebased to use the new logging functions `LogInfo` and `LogDebug` from #28318 (see `developer-notes.md`).
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1898145002)
Rebased to use the new logging functions `LogInfo` and `LogDebug` from #28318 (see `developer-notes.md`).
💬 glozow commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1457194322)
nit: replace magic number with `MAX_STANDARD_TX_WEIGHT`
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1457194322)
nit: replace magic number with `MAX_STANDARD_TX_WEIGHT`
💬 glozow commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1457200003)
Would this be giving up too early? We could still find a different solution further down the function, no?
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1457200003)
Would this be giving up too early? We could still find a different solution further down the function, no?
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-1898164704)
Hm the CI failure of `previous releases, qt5 dev package and depends packages, DEBUG` (CI job used to find silent-merge conflicts (?)) four days ago seems weird. Didn't see any conflicts when rebasing.
Anyway, rebased. Sorry for invalidating your ACK again, @jb55. Fixed the discurage -> discourage misspelling. I thought I did it in an earlier push.
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-1898164704)
Hm the CI failure of `previous releases, qt5 dev package and depends packages, DEBUG` (CI job used to find silent-merge conflicts (?)) four days ago seems weird. Didn't see any conflicts when rebasing.
Anyway, rebased. Sorry for invalidating your ACK again, @jb55. Fixed the discurage -> discourage misspelling. I thought I did it in an earlier push.