💬 achow101 commented on pull request "wallet: Fix use-after-free in WalletBatch::EraseRecords":
(https://github.com/bitcoin/bitcoin/pull/29176#issuecomment-1877254328)
ACK faebf1df2afe207f5d2d4f73f50ac66824fe34bb
(https://github.com/bitcoin/bitcoin/pull/29176#issuecomment-1877254328)
ACK faebf1df2afe207f5d2d4f73f50ac66824fe34bb
🚀 achow101 merged a pull request: "wallet: Fix use-after-free in WalletBatch::EraseRecords"
(https://github.com/bitcoin/bitcoin/pull/29176)
(https://github.com/bitcoin/bitcoin/pull/29176)
👍 real-or-random approved a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/29169#pullrequestreview-1804417620)
Concept ACK https://github.com/bitcoin/bitcoin/commit/c13a17c6996442f04635bdf70ee8f06bf6854ff6 I haven't checked that the subtree is correct but no objections to update to this commit
(https://github.com/bitcoin/bitcoin/pull/29169#pullrequestreview-1804417620)
Concept ACK https://github.com/bitcoin/bitcoin/commit/c13a17c6996442f04635bdf70ee8f06bf6854ff6 I haven't checked that the subtree is correct but no objections to update to this commit
💬 fanquake commented on pull request "wallet: Fix use-after-free in WalletBatch::EraseRecords":
(https://github.com/bitcoin/bitcoin/pull/29176#issuecomment-1877306996)
Backported to 26.x in #29011.
(https://github.com/bitcoin/bitcoin/pull/29176#issuecomment-1877306996)
Backported to 26.x in #29011.
🤔 jonasnick reviewed a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/29169#pullrequestreview-1804437226)
reACK e2cdeb592596432039d21f4c819d45f1e46d65ef
(https://github.com/bitcoin/bitcoin/pull/29169#pullrequestreview-1804437226)
reACK e2cdeb592596432039d21f4c819d45f1e46d65ef
💬 desi-bitcoiner commented on pull request "doc: revert clarify -datacarriersize":
(https://github.com/bitcoin/bitcoin/pull/29173#issuecomment-1877319214)
It's a disgrace that core devs with merge rights are behaving as kids and not taking this PR seriously. Please open this issue again and address the concerns with valid arguments rather than acting tyrannical and closing the issue.
(https://github.com/bitcoin/bitcoin/pull/29173#issuecomment-1877319214)
It's a disgrace that core devs with merge rights are behaving as kids and not taking this PR seriously. Please open this issue again and address the concerns with valid arguments rather than acting tyrannical and closing the issue.
💬 stratospher commented on pull request "test: Use test framework utils in functional tests":
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441924369)
2103e12: here and for `assert_less_than_or_equal` too.
```suggestion
raise AssertionError("%s >= %s" % (str(thing1), str(thing2)))
```
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441924369)
2103e12: here and for `assert_less_than_or_equal` too.
```suggestion
raise AssertionError("%s >= %s" % (str(thing1), str(thing2)))
```
💬 stratospher commented on pull request "test: Use test framework utils in functional tests":
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441930203)
2103e12: maybe just have `assert_not_equal(thing1, thing2)`. i couldn't find an occurrence where we needed to `assert_not_equal` 3+ things. ex: `assert_not_equal(2, 2, 3)`
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441930203)
2103e12: maybe just have `assert_not_equal(thing1, thing2)`. i couldn't find an occurrence where we needed to `assert_not_equal` 3+ things. ex: `assert_not_equal(2, 2, 3)`
💬 desi-bitcoiner commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1877342635)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1877342635)
Concept ACK
💬 stratospher commented on pull request "test: Use test framework utils in functional tests":
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441947958)
i think you can find more places where `assert_not_equal` can be used using the command @theStack mentioned [here](https://github.com/bitcoin/bitcoin/issues/23119#issue-1009729239)!
example first 3 lines output:
```
$ git grep "assert ".*\!=
feature_fee_estimation.py: assert fee_dat_current_content != fee_dat_initial_content
feature_fee_estimation.py: assert fee_dat_current_content != fee_dat_initial_content
mempool_accept_wtxid.py: assert child_one_wtxid != child_tw
...
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441947958)
i think you can find more places where `assert_not_equal` can be used using the command @theStack mentioned [here](https://github.com/bitcoin/bitcoin/issues/23119#issue-1009729239)!
example first 3 lines output:
```
$ git grep "assert ".*\!=
feature_fee_estimation.py: assert fee_dat_current_content != fee_dat_initial_content
feature_fee_estimation.py: assert fee_dat_current_content != fee_dat_initial_content
mempool_accept_wtxid.py: assert child_one_wtxid != child_tw
...
💬 maflcko commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1877382331)
lgtm ACK 842c288852c4d66de359e6907d9c82b7e618ab65
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1877382331)
lgtm ACK 842c288852c4d66de359e6907d9c82b7e618ab65
👍 TheCharlatan approved a pull request: "[26.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29011#pullrequestreview-1804535841)
ACK 7b79e54474b86864c81148c74824bfe4b732412d
(https://github.com/bitcoin/bitcoin/pull/29011#pullrequestreview-1804535841)
ACK 7b79e54474b86864c81148c74824bfe4b732412d
🤔 aureleoules reviewed a pull request: "wallet, rpc: `FundTransaction` refactor"
(https://github.com/bitcoin/bitcoin/pull/28560#pullrequestreview-1804531120)
A few nits
(https://github.com/bitcoin/bitcoin/pull/28560#pullrequestreview-1804531120)
A few nits
💬 aureleoules commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441984071)
```suggestion
if (sffo_set.contains(pos))
```
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441984071)
```suggestion
if (sffo_set.contains(pos))
```
💬 aureleoules commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441983854)
```suggestion
CRecipient recipient = {destination, amount, subtract_fee_outputs.contains(idx)};
```
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441983854)
```suggestion
CRecipient recipient = {destination, amount, subtract_fee_outputs.contains(idx)};
```
💬 aureleoules commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441984340)
```suggestion
if (recipients.empty())
```
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441984340)
```suggestion
if (recipients.empty())
```
📝 glozow opened a pull request: "test: wallet rescan with reorged parent + IsFromMe child in mempool"
(https://github.com/bitcoin/bitcoin/pull/29179)
Originally motivated by #29019, which reverts back to having `requestMempoolTransactions` emit `transactionAddedToMempool` in `mapTx` default order instead of `GetSortedDepthAndScore` order.
It's important that these notifications happen in topological order, otherwise the wallet rescan may miss transactions that belong to it. Notably, checking whether a transaction `IsFromMe` requires knowing its inputs, which may be from a mempool parent.
When using `mapTx` order, a parent may come later
...
(https://github.com/bitcoin/bitcoin/pull/29179)
Originally motivated by #29019, which reverts back to having `requestMempoolTransactions` emit `transactionAddedToMempool` in `mapTx` default order instead of `GetSortedDepthAndScore` order.
It's important that these notifications happen in topological order, otherwise the wallet rescan may miss transactions that belong to it. Notably, checking whether a transaction `IsFromMe` requires knowing its inputs, which may be from a mempool parent.
When using `mapTx` order, a parent may come later
...
💬 hebasto commented on issue "ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc":
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1877397067)
Should we use the [`-mstrict-align`](https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html#index-mstrict-align) option?
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1877397067)
Should we use the [`-mstrict-align`](https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html#index-mstrict-align) option?
💬 glozow commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1877401757)
> Will this PR not cause issues [here](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet.cpp#L1938)? In [`CWallet::transactionAddedToMempool`](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet.cpp#L1406), we eventually call `CWallet::AddToWalletIfInvolvingMe` where we have a [dependency](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet
...
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1877401757)
> Will this PR not cause issues [here](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet.cpp#L1938)? In [`CWallet::transactionAddedToMempool`](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet.cpp#L1406), we eventually call `CWallet::AddToWalletIfInvolvingMe` where we have a [dependency](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet
...
💬 darosior commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1877414034)
Can a maintainer please close this PR? It's clear this is not going anywhere and is just **spamming** the repository.
Most regular contributors to this project who gave their opinion here and elsewhere are against making this change. Nothing's preventing people interested in this code change to fork the project and develop their own Bitcoin P2P client. (As a matter of fact OP is already maintaining such a client, people interested in this could contribute to this project instead of arguing he
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1877414034)
Can a maintainer please close this PR? It's clear this is not going anywhere and is just **spamming** the repository.
Most regular contributors to this project who gave their opinion here and elsewhere are against making this change. Nothing's preventing people interested in this code change to fork the project and develop their own Bitcoin P2P client. (As a matter of fact OP is already maintaining such a client, people interested in this could contribute to this project instead of arguing he
...