💬 furszy commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1494577858)
> It seems all these lines of code might be replaced with `ConfirmMessage(nullptr, 500ms);`.
That should trigger the filesystem window to dump the psbt info to a file, which would incur in another step to cancel the dialog. The idea here is to obtain the data through the clipboard only (see few lines below), not through a file.
That being said, agree that we could move this into the utility file in the future and cleanup some code.
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1494577858)
> It seems all these lines of code might be replaced with `ConfirmMessage(nullptr, 500ms);`.
That should trigger the filesystem window to dump the psbt info to a file, which would incur in another step to cancel the dialog. The idea here is to obtain the data through the clipboard only (see few lines below), not through a file.
That being said, agree that we could move this into the utility file in the future and cleanup some code.
💬 glozow commented on pull request "[26.1] final changes for 26.1rc1":
(https://github.com/bitcoin/bitcoin/pull/29440#issuecomment-1952509971)
> (Up to you if you want to fix the fuzzing, I don't need it, I just wanted to mention that the fix is missing)
Seems better to leave it as is
(https://github.com/bitcoin/bitcoin/pull/29440#issuecomment-1952509971)
> (Up to you if you want to fix the fuzzing, I don't need it, I just wanted to mention that the fix is missing)
Seems better to leave it as is
📝 hebasto opened a pull request: "test: Recognize dialog object by name"
(https://github.com/bitcoin-core/gui/pull/797)
Fixes https://github.com/bitcoin-core/gui/issues/796.
(https://github.com/bitcoin-core/gui/pull/797)
Fixes https://github.com/bitcoin-core/gui/issues/796.
💬 hebasto commented on pull request "test: Recognize dialog object by name":
(https://github.com/bitcoin-core/gui/pull/797#issuecomment-1952522066)
cc @maflcko @furszy
(https://github.com/bitcoin-core/gui/pull/797#issuecomment-1952522066)
cc @maflcko @furszy
💬 furszy commented on issue "ci_native_asan: UndefinedBehaviorSanitizer: null-pointer-use qt/test/wallettests.cpp:424:25 in":
(https://github.com/bitcoin-core/gui/issues/796#issuecomment-1952529363)
It is because the test is trying to access the `QMessageBox::Discard` button on a dialog that does not have a discard button. It does not happen regularly because `SendCoins()` usually confirms the `SendConfirmationDialog` dialog in less than 500ms, which destructs the dialog, only leaving the psbt dialog active by the time the timer is triggered.
(https://github.com/bitcoin-core/gui/issues/796#issuecomment-1952529363)
It is because the test is trying to access the `QMessageBox::Discard` button on a dialog that does not have a discard button. It does not happen regularly because `SendCoins()` usually confirms the `SendConfirmationDialog` dialog in less than 500ms, which destructs the dialog, only leaving the psbt dialog active by the time the timer is triggered.
🤔 furszy reviewed a pull request: "test: Recognize dialog object by name"
(https://github.com/bitcoin-core/gui/pull/797#pullrequestreview-1888508153)
Code ACK 4c9db9b587
(https://github.com/bitcoin-core/gui/pull/797#pullrequestreview-1888508153)
Code ACK 4c9db9b587
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1494616142)
Would need to add a suppression for now?
```
node1 stderr rpc/blockchain.cpp:1658:42: runtime error: unsigned integer overflow: 0 - 200 cannot be represented in type 'unsigned int'
#0 0x5622afcfe34c in getchaintxstats()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const src/rpc/blockchain.cpp:1658:42
#1 0x5622afcfe34c in UniValue std::__invoke_impl<UniValue, getchaintxstats()::$_0&, RPCHelpMan const&, JSONRPCRequest const&>(std::__invoke_other, getchaintxstats()::$_0
...
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1494616142)
Would need to add a suppression for now?
```
node1 stderr rpc/blockchain.cpp:1658:42: runtime error: unsigned integer overflow: 0 - 200 cannot be represented in type 'unsigned int'
#0 0x5622afcfe34c in getchaintxstats()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const src/rpc/blockchain.cpp:1658:42
#1 0x5622afcfe34c in UniValue std::__invoke_impl<UniValue, getchaintxstats()::$_0&, RPCHelpMan const&, JSONRPCRequest const&>(std::__invoke_other, getchaintxstats()::$_0
...
👋 glozow's pull request is ready for review: "policy: enable sibling eviction for v3 transactions"
(https://github.com/bitcoin/bitcoin/pull/29306)
(https://github.com/bitcoin/bitcoin/pull/29306)
💬 hebasto commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1494650375)
> That will trigger the filesystem window to dump the psbt info to a file...
Why? The `ConfirmMessage` triggers the default button, which is `QMessageBox::Discard`, no?
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1494650375)
> That will trigger the filesystem window to dump the psbt info to a file...
Why? The `ConfirmMessage` triggers the default button, which is `QMessageBox::Discard`, no?
👍 fanquake approved a pull request: "[26.1] final changes for 26.1rc1"
(https://github.com/bitcoin/bitcoin/pull/29440#pullrequestreview-1888599554)
ACK 1e7fb270d36310efff6fc968f1c52291043d461b
(https://github.com/bitcoin/bitcoin/pull/29440#pullrequestreview-1888599554)
ACK 1e7fb270d36310efff6fc968f1c52291043d461b
💬 furszy commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1494667687)
> > That will trigger the filesystem window to dump the psbt info to a file...
>
> Why? The `ConfirmMessage` triggers the default button, which is `QMessageBox::Discard`, no?
Yeah ok, I did not see the manual `setDefaultButton` call. That will work fine while the dialog takes less than 500ms in popping up. If it takes longer, the test will stall because `ConfirmMessage` executes runs the time only once (`QTimer::singleShot`).
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1494667687)
> > That will trigger the filesystem window to dump the psbt info to a file...
>
> Why? The `ConfirmMessage` triggers the default button, which is `QMessageBox::Discard`, no?
Yeah ok, I did not see the manual `setDefaultButton` call. That will work fine while the dialog takes less than 500ms in popping up. If it takes longer, the test will stall because `ConfirmMessage` executes runs the time only once (`QTimer::singleShot`).
💬 hebasto commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1952628049)
> Or is it more-so just to keep IWYU usable without warnings for our c-i configs specifically?
Yes. At some point in the future we might want to treat IWYU warnings as errors.
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1952628049)
> Or is it more-so just to keep IWYU usable without warnings for our c-i configs specifically?
Yes. At some point in the future we might want to treat IWYU warnings as errors.
👍 hebasto approved a pull request: "refactor: bitcoin-config.h includes cleanup"
(https://github.com/bitcoin/bitcoin/pull/29404#pullrequestreview-1888622822)
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/29404#pullrequestreview-1888622822)
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK.
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1952644105)
In the context of Stratum v2 #29432 I'm looking for a more efficient way to decide when to generate a new block template.
The current implementation looks at the mempool's `GetTransactionsUpdated()` every `-sv2interval` seconds. If there was any update, it calls `CreateNewBlock()` on `BlockAssembler`. It then checks if fees increased by at least `-sv2feedelta` since the last template and if so, pushes it to connected clients.
`GetTransactionsUpdated()` changes all the time, especially with
...
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1952644105)
In the context of Stratum v2 #29432 I'm looking for a more efficient way to decide when to generate a new block template.
The current implementation looks at the mempool's `GetTransactionsUpdated()` every `-sv2interval` seconds. If there was any update, it calls `CreateNewBlock()` on `BlockAssembler`. It then checks if fees increased by at least `-sv2feedelta` since the last template and if so, pushes it to connected clients.
`GetTransactionsUpdated()` changes all the time, especially with
...
💬 sipa commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1952653016)
> Would it be easy to keep track of that (approximate) number every time something is added or removed from the mempool?
After clustermempool, yes. Today doing that inherently requires running the mining algorithm to figure out what transactions to put there.
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1952653016)
> Would it be easy to keep track of that (approximate) number every time something is added or removed from the mempool?
After clustermempool, yes. Today doing that inherently requires running the mining algorithm to figure out what transactions to put there.
💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1952657689)
Indeed, I meant on top of this PR (or a later incarnation).
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1952657689)
Indeed, I meant on top of this PR (or a later incarnation).
💬 hebasto commented on pull request "ci: Avoid CI failures from temp env file reuse":
(https://github.com/bitcoin/bitcoin/pull/29441#discussion_r1494723144)
This comment got outdated, no?
(https://github.com/bitcoin/bitcoin/pull/29441#discussion_r1494723144)
This comment got outdated, no?
🚀 fanquake merged a pull request: "[26.1] final changes for 26.1rc1"
(https://github.com/bitcoin/bitcoin/pull/29440)
(https://github.com/bitcoin/bitcoin/pull/29440)
💬 gdiscord commented on issue "Guix build script incorrectly reporting there is no Mac SDK":
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1952703183)
sure, will oblige.
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1952703183)
sure, will oblige.
📝 fanquake opened a pull request: "doc: document that BIP324 on by default for v27.0"
(https://github.com/bitcoin/bitcoin/pull/29452)
Addresses: https://github.com/bitcoin/bitcoin/pull/29347#issuecomment-1952335331.
(https://github.com/bitcoin/bitcoin/pull/29452)
Addresses: https://github.com/bitcoin/bitcoin/pull/29347#issuecomment-1952335331.