π¬ davidgumberg commented on pull request "fuzz: Some `test/fuzz/test_runner.py` improvements":
(https://github.com/bitcoin/bitcoin/pull/29821#issuecomment-2041487197)
utACK https://github.com/bitcoin/bitcoin/pull/29821/commits/47cedee776c6253232beb6039ea708c578211327
(https://github.com/bitcoin/bitcoin/pull/29821#issuecomment-2041487197)
utACK https://github.com/bitcoin/bitcoin/pull/29821/commits/47cedee776c6253232beb6039ea708c578211327
π¬ fuelmessenger commented on issue "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows":
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2041570653)
:sparkles: Thanks to the community, there's an estimated bounty value of **$17.74 USD** for a successful merge request of this issue via contributions such as **19.500 MATIC** tokens. Happy coding :grinning:
Details and T&Cs at [joinfuel.io](https://joinfuel.io)
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2041570653)
:sparkles: Thanks to the community, there's an estimated bounty value of **$17.74 USD** for a successful merge request of this issue via contributions such as **19.500 MATIC** tokens. Happy coding :grinning:
Details and T&Cs at [joinfuel.io](https://joinfuel.io)
π¬ naiyoma commented on pull request "test: Cleans some manual checks/drops when using wait_for_getdata":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2041579536)
Concept ACK,
nit: Could you also take a look at `announce_tx_and_wait_for_getdata` in `p2p_segwit.py` for similar cleanup?
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2041579536)
Concept ACK,
nit: Could you also take a look at `announce_tx_and_wait_for_getdata` in `p2p_segwit.py` for similar cleanup?
π¬ pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2041615767)
> I don't know how useful this is. Why would users care to see only a certain type, or to see what type addresses are like this?
Well, since QT offers to the user the feature to create certain types, it'd make sense to list them properly identifying the original intention.
<details>
<summary>Personally, I'd add the column on the "<i>Requested payments history</i>" as well (or making it optional, adding previously a feature to show/ hide columns, feature that could be more useful on the Pee
...
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2041615767)
> I don't know how useful this is. Why would users care to see only a certain type, or to see what type addresses are like this?
Well, since QT offers to the user the feature to create certain types, it'd make sense to list them properly identifying the original intention.
<details>
<summary>Personally, I'd add the column on the "<i>Requested payments history</i>" as well (or making it optional, adding previously a feature to show/ hide columns, feature that could be more useful on the Pee
...
π¬ davidgumberg commented on pull request "validation: Use witness maleation flag for non-segwit blocks":
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2041851369)
Concept ACK
Nits that seem to be close to the scope of this PR:
I think this comment is incorrect:
```cpp
/*
* Note: If the witness commitment is expected (i.e. `expect_witness_commitment
* = true`), then the block is required to have at least one transaction and the
* first transaction needs to have at least one input. */
```
Isn't it only if the witness commitment is expected *and* present (i.e. `commitpos != NO_WITNESS_COMMITMENT` ) that:
```cpp
assert(!block.vtx.empt
...
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2041851369)
Concept ACK
Nits that seem to be close to the scope of this PR:
I think this comment is incorrect:
```cpp
/*
* Note: If the witness commitment is expected (i.e. `expect_witness_commitment
* = true`), then the block is required to have at least one transaction and the
* first transaction needs to have at least one input. */
```
Isn't it only if the witness commitment is expected *and* present (i.e. `commitpos != NO_WITNESS_COMMITMENT` ) that:
```cpp
assert(!block.vtx.empt
...
π¬ achow101 commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2042153543)
I'm not really seeing the same issue on my WIndows machine, although IBD is generally a lot slower
(50 runs each, sync to 120,000)
* On 26.1:
* Median 359.9265245
* Mean 374.7166118
* Std. Dev. 24.05475889010843
* On 27.0rc1:
* Median 375.675126
* Mean 375.79323382
* Std. Dev. 2.7244149066583287
Considering that this is rather difficult to figure out what is wrong, and it seems to not be happening for everyone, I think it would be okay for us to move forward with th
...
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2042153543)
I'm not really seeing the same issue on my WIndows machine, although IBD is generally a lot slower
(50 runs each, sync to 120,000)
* On 26.1:
* Median 359.9265245
* Mean 374.7166118
* Std. Dev. 24.05475889010843
* On 27.0rc1:
* Median 375.675126
* Mean 375.79323382
* Std. Dev. 2.7244149066583287
Considering that this is rather difficult to figure out what is wrong, and it seems to not be happening for everyone, I think it would be okay for us to move forward with th
...
π€ glozow reviewed a pull request: "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)"
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-1985578851)
concept ACK thanks! Maybe p2p_invalid_tx.py would be a good file but no strong opinion
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-1985578851)
concept ACK thanks! Maybe p2p_invalid_tx.py would be a good file but no strong opinion
π¬ glozow commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1555316078)
I use `assert_debug_log` a lot in manual testing, but prefer not to in functional tests - logs aren't supposed to be a stable interface (this would break if we change it) and they don't really test the actual behavior we want. In this case, testing that the tx is requested is sufficient to me so we could just get rid of the `with`.
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1555316078)
I use `assert_debug_log` a lot in manual testing, but prefer not to in functional tests - logs aren't supposed to be a stable interface (this would break if we change it) and they don't really test the actual behavior we want. In this case, testing that the tx is requested is sufficient to me so we could just get rid of the `with`.
π¬ glozow commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1555310358)
nit: maybe `test_rejects_filter_rest`
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1555310358)
nit: maybe `test_rejects_filter_rest`
π¬ glozow commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1555420823)
Btw if you wanted to hit the problem in https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553911167, we would want the error to be "mempool min fee not met" (would need `fill_mempool`)
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1555420823)
Btw if you wanted to hit the problem in https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553911167, we would want the error to be "mempool min fee not met" (would need `fill_mempool`)
π rkrux approved a pull request: "test: refactor: introduce and use `calculate_input_weight` helper"
(https://github.com/bitcoin/bitcoin/pull/29777#pullrequestreview-1985788449)
tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390)
1. Successful Make
2. Successful Functional Tests
Thanks for extracting out the common function, and reducing code duplication.
(https://github.com/bitcoin/bitcoin/pull/29777#pullrequestreview-1985788449)
tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390)
1. Successful Make
2. Successful Functional Tests
Thanks for extracting out the common function, and reducing code duplication.
β
achow101 closed a pull request: "wallet: Receive silent payment transactions"
(https://github.com/bitcoin/bitcoin/pull/28453)
(https://github.com/bitcoin/bitcoin/pull/28453)
π¬ achow101 commented on pull request "wallet: Receive silent payment transactions":
(https://github.com/bitcoin/bitcoin/pull/28453#issuecomment-2042181326)
Closing for now as up for grabs as I don't have time to work on this.
With having a silent payments module in libsecp, and also the plan to change this to using a descriptor, a lot of this PR is also no longer relevant.
(https://github.com/bitcoin/bitcoin/pull/28453#issuecomment-2042181326)
Closing for now as up for grabs as I don't have time to work on this.
With having a silent payments module in libsecp, and also the plan to change this to using a descriptor, a lot of this PR is also no longer relevant.
β οΈ Wang-jiong-han opened an issue: "Where is the transactionAddedToMempool function ultimately interpreted"
(https://github.com/bitcoin/bitcoin/issues/29829)
### Motivation
when I read and check the code of AcceptSingleTransaction function,I noticed that the transactionAddedToMempool function is not being constructed properly; existing code related to the transactionAddedToMempool function is either overloaded or repeatedly called. I haven't found where this functionality is ultimately interpreted and executedγ
Is it that I'm missing some part of the code, or that some third-party library is being called? I hope to get an answer from the developer
...
(https://github.com/bitcoin/bitcoin/issues/29829)
### Motivation
when I read and check the code of AcceptSingleTransaction function,I noticed that the transactionAddedToMempool function is not being constructed properly; existing code related to the transactionAddedToMempool function is either overloaded or repeatedly called. I haven't found where this functionality is ultimately interpreted and executedγ
Is it that I'm missing some part of the code, or that some third-party library is being called? I hope to get an answer from the developer
...
π achow101 opened a pull request: "doc: 25.2 historical release notes"
(https://github.com/bitcoin/bitcoin/pull/29830)
(https://github.com/bitcoin/bitcoin/pull/29830)
π¬ brunoerg commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1555541188)
In 9297437af2c2f45a7b9a12be78fa0e597c54ac79: Since the addrman is deterministic now, you can remove addresses that are conflicting.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1555541188)
In 9297437af2c2f45a7b9a12be78fa0e597c54ac79: Since the addrman is deterministic now, you can remove addresses that are conflicting.
β
glozow closed an issue: "Where is the transactionAddedToMempool function ultimately interpreted"
(https://github.com/bitcoin/bitcoin/issues/29829)
(https://github.com/bitcoin/bitcoin/issues/29829)
π¬ glozow commented on issue "Where is the transactionAddedToMempool function ultimately interpreted":
(https://github.com/bitcoin/bitcoin/issues/29829#issuecomment-2042329403)
This seems like a misunderstanding of how `CValidationInterface` works? https://github.com/bitcoin/bitcoin/blob/master/src/validationinterface.h. Clients of that interface implement `transactionAddedToMempool`.
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) or the #bitcoin IRC channel on Libera Chat, or on
...
(https://github.com/bitcoin/bitcoin/issues/29829#issuecomment-2042329403)
This seems like a misunderstanding of how `CValidationInterface` works? https://github.com/bitcoin/bitcoin/blob/master/src/validationinterface.h. Clients of that interface implement `transactionAddedToMempool`.
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) or the #bitcoin IRC channel on Libera Chat, or on
...
π rkrux approved a pull request: "test: fix accurate multisig sigop count (BIP16), add unit test"
(https://github.com/bitcoin/bitcoin/pull/29615#pullrequestreview-1985960918)
tACK [3e9c736](https://github.com/bitcoin/bitcoin/pull/29615/commits/3e9c736a26724ffe3b70b387995fbf48c06300e2)
1. Make successful
2. Functional tests successful
Asked couple questions for my clarity.
(https://github.com/bitcoin/bitcoin/pull/29615#pullrequestreview-1985960918)
tACK [3e9c736](https://github.com/bitcoin/bitcoin/pull/29615/commits/3e9c736a26724ffe3b70b387995fbf48c06300e2)
1. Make successful
2. Functional tests successful
Asked couple questions for my clarity.
π¬ rkrux commented on pull request "test: fix accurate multisig sigop count (BIP16), add unit test":
(https://github.com/bitcoin/bitcoin/pull/29615#discussion_r1555548944)
Would you know why the function accepts a `fAccurate` boolean?
The only occurrence of `fAccurate = false` is in `feature_taproot.py`, rest of them pass `true` by default.
(https://github.com/bitcoin/bitcoin/pull/29615#discussion_r1555548944)
Would you know why the function accepts a `fAccurate` boolean?
The only occurrence of `fAccurate = false` is in `feature_taproot.py`, rest of them pass `true` by default.