💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1548540109)
```suggestion
* - Get a transaction for broadcast, the one that has been broadcast fewer times and least recently
```
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1548540109)
```suggestion
* - Get a transaction for broadcast, the one that has been broadcast fewer times and least recently
```
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1548580964)
nit: This could be `const` method?
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1548580964)
nit: This could be `const` method?
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1548596412)
Since we're touching these lines, maybe update to `LogPrintLevel`?
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1548596412)
Since we're touching these lines, maybe update to `LogPrintLevel`?
💬 tdb3 commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2033180778)
Thank you for taking a look @Sjors and @maflcko!
> If you run these in parallel, I think you might as well drop the `--skipunit` since it's faster than many of the other tests.
To ensure I understand, is the desire here to have the unit tests run by default when running individual tests (and also by default when running all tests)?
If so, that seems reasonable to me. As you mentioned, the unit tests are decently fast and the default job value is 4 anyway (so there would be some inheren
...
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2033180778)
Thank you for taking a look @Sjors and @maflcko!
> If you run these in parallel, I think you might as well drop the `--skipunit` since it's faster than many of the other tests.
To ensure I understand, is the desire here to have the unit tests run by default when running individual tests (and also by default when running all tests)?
If so, that seems reasonable to me. As you mentioned, the unit tests are decently fast and the default job value is 4 anyway (so there would be some inheren
...
📝 achow101 opened a pull request: "[25.x] Finalize 25.2"
(https://github.com/bitcoin/bitcoin/pull/29794)
Final changes for 25.2 release
(https://github.com/bitcoin/bitcoin/pull/29794)
Final changes for 25.2 release
🤔 marcofleon reviewed a pull request: "cli: Detect port errors in rpcconnect and rpcport"
(https://github.com/bitcoin/bitcoin/pull/29521#pullrequestreview-1975216076)
Tested ACK. The new error handling code looks good to me upon review. Ran the daemon and gave invalid values for `rpcconnect` and `rpcport` and the proper error messages came up. The behavior of port 0 being invalid is consistent with `init.cpp`. Ran `interface_bitcoin_cli.py` too and there were no issues with the added tests.
(https://github.com/bitcoin/bitcoin/pull/29521#pullrequestreview-1975216076)
Tested ACK. The new error handling code looks good to me upon review. Ran the daemon and gave invalid values for `rpcconnect` and `rpcport` and the proper error messages came up. The behavior of port 0 being invalid is consistent with `init.cpp`. Ran `interface_bitcoin_cli.py` too and there were no issues with the added tests.
💬 tdb3 commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2033263056)
> Tested ACK. The new error handling code looks good to me upon review. Ran the daemon and gave invalid values for `rpcconnect` and `rpcport` and the proper error messages came up. The behavior of port 0 being invalid is consistent with `src/init.cpp`. Ran `interface_bitcoin_cli.py` too and there were no issues with the added tests.
Thank you for reviewing, @marcofleon!
(https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2033263056)
> Tested ACK. The new error handling code looks good to me upon review. Ran the daemon and gave invalid values for `rpcconnect` and `rpcport` and the proper error messages came up. The behavior of port 0 being invalid is consistent with `src/init.cpp`. Ran `interface_bitcoin_cli.py` too and there were no issues with the added tests.
Thank you for reviewing, @marcofleon!
💬 pablomartin4btc commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1548752403)
**nit**: the tx won't be created when an error is raised during the `createTransaction`'s call (eg no recipient) so the obj should represent the correct thing, unless I misunderstanding the use case here... ok I see now this is how `src/util/result.h` works, so ignore my correction, I find it a bit weird but perhaps cos I'm not used to it yet. As a ref this change could be useful https://github.com/bitcoin/bitcoin/pull/25665
```suggestion
virtual util::Result<CreateTransactionResult> crea
...
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1548752403)
**nit**: the tx won't be created when an error is raised during the `createTransaction`'s call (eg no recipient) so the obj should represent the correct thing, unless I misunderstanding the use case here... ok I see now this is how `src/util/result.h` works, so ignore my correction, I find it a bit weird but perhaps cos I'm not used to it yet. As a ref this change could be useful https://github.com/bitcoin/bitcoin/pull/25665
```suggestion
virtual util::Result<CreateTransactionResult> crea
...
👍 pablomartin4btc approved a pull request: "refactor: interfaces, make 'createTransaction' less error-prone "
(https://github.com/bitcoin-core/gui/pull/807#pullrequestreview-1975255350)
cr ACK dfbd145676255f82bad529976928dff1939c5e48
(https://github.com/bitcoin-core/gui/pull/807#pullrequestreview-1975255350)
cr ACK dfbd145676255f82bad529976928dff1939c5e48
💬 davidgumberg commented on pull request "lint: Rewrite spelling, includes, and include guards linters in Rust":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2033339070)
I've substantially revised this PR to address @fjahr's feedback that the rust lint runner monofile was getting too big, and that it doesn't support running individual tests (https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2022796657) and @maflcko's suggestion to include `test/lint/lint-locale-dependence.py` and `test/lint/lint-python-utf8-encoding.py` (https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2022606314).
I do not feel confident in regex, and had to do some tricky
...
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2033339070)
I've substantially revised this PR to address @fjahr's feedback that the rust lint runner monofile was getting too big, and that it doesn't support running individual tests (https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2022796657) and @maflcko's suggestion to include `test/lint/lint-locale-dependence.py` and `test/lint/lint-python-utf8-encoding.py` (https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2022606314).
I do not feel confident in regex, and had to do some tricky
...
💬 kevkevinpal commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548842239)
nit: not sure if this comment is redundant now since the function name is `calculate_input_weight`
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548842239)
nit: not sure if this comment is redundant now since the function name is `calculate_input_weight`
💬 kevkevinpal commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548842910)
nit: not sure if this comment is redundant now since the function name is `calculate_input_weight`
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548842910)
nit: not sure if this comment is redundant now since the function name is `calculate_input_weight`
💬 kevkevinpal commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548845139)
It might make sense to add another test that does not explicitly set `witness_stack_hex` equal to `None`
```
self.assertEqual(calculate_input_weight(""),
(SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR)
```
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548845139)
It might make sense to add another test that does not explicitly set `witness_stack_hex` equal to `None`
```
self.assertEqual(calculate_input_weight(""),
(SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR)
```
💬 kevkevinpal commented on pull request "test: refactor: introduce and use `calculate_input_weight` helper":
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548863105)
nit: we only need to define this once
(https://github.com/bitcoin/bitcoin/pull/29777#discussion_r1548863105)
nit: we only need to define this once
💬 pablomartin4btc commented on pull request "Fix create unsigned transaction fee bump":
(https://github.com/bitcoin-core/gui/pull/812#discussion_r1548912316)
At least on Ubuntu 22.04, now after the fee-bumping unsigned a modal pop-up is displayed:

while before there was a message sent thru the OS system information (?):

I see in the code we usually use `CClientUIInterface::MSG_INFORMATION` (`transactionview.cpp`, `walletview.cpp`) and I'd pre
...
(https://github.com/bitcoin-core/gui/pull/812#discussion_r1548912316)
At least on Ubuntu 22.04, now after the fee-bumping unsigned a modal pop-up is displayed:

while before there was a message sent thru the OS system information (?):

I see in the code we usually use `CClientUIInterface::MSG_INFORMATION` (`transactionview.cpp`, `walletview.cpp`) and I'd pre
...
👍 pablomartin4btc approved a pull request: "Fix create unsigned transaction fee bump"
(https://github.com/bitcoin-core/gui/pull/812#pullrequestreview-1975499213)
tACK 671b7a32516d62e1e79393ded4b45910bd08010a
Tested and verified this PR fixes #810 and now creation of unsigned tx/ PSBT/ during fee-bumping doesn't ask for passphrase/ requires wallet unlock.
(https://github.com/bitcoin-core/gui/pull/812#pullrequestreview-1975499213)
tACK 671b7a32516d62e1e79393ded4b45910bd08010a
Tested and verified this PR fixes #810 and now creation of unsigned tx/ PSBT/ during fee-bumping doesn't ask for passphrase/ requires wallet unlock.
👍 pablomartin4btc approved a pull request: "Change example address from legacy (P2PKH) to bech32m (P2TR)"
(https://github.com/bitcoin-core/gui/pull/808#pullrequestreview-1975550497)
tACK c6d1b8de89d87fe4fd171dc85557299e429e6564
<details>
<summary>Tested running <code>bitcoin-qt</code> on <code>signet</code>.</summary>

</details>
<details>
<summary>Also tested the script provided in the <a href="https://github.com/bitcoin-core/gui/pull/808#issue-2202921882">description</a>.</summary>
```
/test/functional$ ./gen_add_type.py
mainnet example address: bc1p35yvje
...
(https://github.com/bitcoin-core/gui/pull/808#pullrequestreview-1975550497)
tACK c6d1b8de89d87fe4fd171dc85557299e429e6564
<details>
<summary>Tested running <code>bitcoin-qt</code> on <code>signet</code>.</summary>

</details>
<details>
<summary>Also tested the script provided in the <a href="https://github.com/bitcoin-core/gui/pull/808#issue-2202921882">description</a>.</summary>
```
/test/functional$ ./gen_add_type.py
mainnet example address: bc1p35yvje
...
💬 0xB10C commented on pull request "ci: Temporarily disable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2033694468)
I'm missing context for this change. Why no PR description and no commit message body?
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2033694468)
I'm missing context for this change. Why no PR description and no commit message body?
⚠️ pinwhell opened an issue: "Concern: Transatlantic Internet Seizure, Impact on Bitcoin Transactions & Blockchain Forks"
(https://github.com/bitcoin/bitcoin/issues/29795)
How could a prolonged transatlantic internet disruption, such as one triggered by war or sabotage, impact Bitcoin transactions and user funds? Specifically, what might occur in terms of a potential blockchain split or fork, and how could efforts to reconcile such a situation be affected?
(https://github.com/bitcoin/bitcoin/issues/29795)
How could a prolonged transatlantic internet disruption, such as one triggered by war or sabotage, impact Bitcoin transactions and user funds? Specifically, what might occur in terms of a potential blockchain split or fork, and how could efforts to reconcile such a situation be affected?
✅ maflcko closed an issue: "Concern: Transatlantic Internet Seizure, Impact on Bitcoin Transactions & Blockchain Forks"
(https://github.com/bitcoin/bitcoin/issues/29795)
(https://github.com/bitcoin/bitcoin/issues/29795)