💬 instagibbs commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2901247824)
Also wondering aloud how we can accurately test deep reorgs on real networks, like a mainnet node for performance testing. Anyone have suggestions on this?
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2901247824)
Also wondering aloud how we can accurately test deep reorgs on real networks, like a mainnet node for performance testing. Anyone have suggestions on this?
💬 laanwj commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901265062)
> Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using DEBUG=1? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
In the short term, getting the CI to pass reliably again is most important imo. Adding another DEBUG run can always be considered, but shouldn't come at the cost of CI flakiness.
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901265062)
> Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using DEBUG=1? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
In the short term, getting the CI to pass reliably again is most important imo. Adding another DEBUG run can always be considered, but shouldn't come at the cost of CI flakiness.
💬 romanz commented on issue "Support `Accept` HTTP header in REST API":
(https://github.com/bitcoin/bitcoin/issues/32583#issuecomment-2901271892)
Thanks @maflcko - you're right, it's better to have one way for specifying the expected content format.
@yancyribbens WDYT?
(https://github.com/bitcoin/bitcoin/issues/32583#issuecomment-2901271892)
Thanks @maflcko - you're right, it's better to have one way for specifying the expected content format.
@yancyribbens WDYT?
💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2102633359)
> programs
thx, I may address this, if I have to re-touch for other reasons
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2102633359)
> programs
thx, I may address this, if I have to re-touch for other reasons
📝 fanquake opened a pull request: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32589)
Backports
- #32551 (just 800b7cc42ca63f2a6b245a4d327c7092289da6e1)
(https://github.com/bitcoin/bitcoin/pull/32589)
Backports
- #32551 (just 800b7cc42ca63f2a6b245a4d327c7092289da6e1)
💬 instagibbs commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#discussion_r2102559718)
Took me a minute to realize this is only the 2nd(?) case where it's valid in block but non-standard-even-with-acceotnonstdtxn set.
Also meta: would be nice if there was a border testing capability, where you could pair it with a tx that would be successful and obviously just-at the limit.
(https://github.com/bitcoin/bitcoin/pull/32533#discussion_r2102559718)
Took me a minute to realize this is only the 2nd(?) case where it's valid in block but non-standard-even-with-acceotnonstdtxn set.
Also meta: would be nice if there was a border testing capability, where you could pair it with a tx that would be successful and obviously just-at the limit.
💬 instagibbs commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#discussion_r2102602017)
```Suggestion
MAX_STANDARD_TX_SIGOPS = MAX_BLOCK_SIGOPS/5
```
(https://github.com/bitcoin/bitcoin/pull/32533#discussion_r2102602017)
```Suggestion
MAX_STANDARD_TX_SIGOPS = MAX_BLOCK_SIGOPS/5
```
👍 instagibbs approved a pull request: "test: properly check for per-tx sigops limit"
(https://github.com/bitcoin/bitcoin/pull/32533#pullrequestreview-2861227732)
crACK 7bc64a8859c3644fdf2eeff59f72a778ae60ea3f
just nits you can ignore
(https://github.com/bitcoin/bitcoin/pull/32533#pullrequestreview-2861227732)
crACK 7bc64a8859c3644fdf2eeff59f72a778ae60ea3f
just nits you can ignore
💬 fanquake commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2901333121)
Backported to 29.x in #32589.
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2901333121)
Backported to 29.x in #32589.
🚀 glozow merged a pull request: "test: properly check for per-tx sigops limit"
(https://github.com/bitcoin/bitcoin/pull/32533)
(https://github.com/bitcoin/bitcoin/pull/32533)
💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2901371140)
Approach ACK fa53098472 (still reviewing code changes around blocktip notifications)
With fresh brain this morning I think this is the best possible approach if we make any change at all. The regtest stuff I observed is expected behavior - if there's no block for 2 days then yes you are 288 blocks behind and progress should estimate that. The new chainparams define regtest as having an expected 0.001 tx/s which is approximately equal to one (coinbase) tx every ten minutes.
Example catchi
...
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2901371140)
Approach ACK fa53098472 (still reviewing code changes around blocktip notifications)
With fresh brain this morning I think this is the best possible approach if we make any change at all. The regtest stuff I observed is expected behavior - if there's no block for 2 days then yes you are 288 blocks behind and progress should estimate that. The new chainparams define regtest as having an expected 0.001 tx/s which is approximately equal to one (coinbase) tx every ten minutes.
Example catchi
...
💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102669940)
Just to avoid to take the lock again. You suggestion is also fine. Happy to push it, just let me know.
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102669940)
Just to avoid to take the lock again. You suggestion is also fine. Happy to push it, just let me know.
💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102672146)
the lock is recursive and all callers already held it, except for one, which is handled in the next commit: https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2100887156
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102672146)
the lock is recursive and all callers already held it, except for one, which is handled in the next commit: https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2100887156
💬 pinheadmz commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102678855)
Ah thanks, good justification, thats good 👍
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2102678855)
Ah thanks, good justification, thats good 👍
💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2901391171)
> > I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
>
> Hmm the shell support implemented in [eb16060](https://github.com/bitcoin/bitcoin/commit/eb160602a50bebcca3f53cdae741e1732738d51a) actually seems somewhat broken as it is still splitting and joining instead of preserving the original string passed to Popen() instead of just passing it unaltered to execvp as I was suggesting.
Thanks! Implemented as you suggested.
(no further clean up
...
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2901391171)
> > I've restored the `shell` option in the `subprocess` API, and employed it on non-Windows systems.
>
> Hmm the shell support implemented in [eb16060](https://github.com/bitcoin/bitcoin/commit/eb160602a50bebcca3f53cdae741e1732738d51a) actually seems somewhat broken as it is still splitting and joining instead of preserving the original string passed to Popen() instead of just passing it unaltered to execvp as I was suggesting.
Thanks! Implemented as you suggested.
(no further clean up
...
⚠️ fanquake opened an issue: "seeds: seed.bitcoin.jonasschnelli.ch not returning results"
(https://github.com/bitcoin/bitcoin/issues/32590)
Also shown by [check-dnsseeds.py](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py):
```bash
./check-dnsseeds.py
* mainnet
OK seed.bitcoin.sipa.be (39 results)
OK dnsseed.bluematt.me (31 results)
OK dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us (35 results)
OK seed.bitcoinstats.com (24 results)
FAIL seed.bitcoin.jonasschnelli.ch
OK seed.btc.petertodd.net (37 results)
OK seed.bitcoin.sprovoost.nl (36 results)
OK dnsseed.emzy.de (40 results)
OK see
...
(https://github.com/bitcoin/bitcoin/issues/32590)
Also shown by [check-dnsseeds.py](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py):
```bash
./check-dnsseeds.py
* mainnet
OK seed.bitcoin.sipa.be (39 results)
OK dnsseed.bluematt.me (31 results)
OK dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us (35 results)
OK seed.bitcoinstats.com (24 results)
FAIL seed.bitcoin.jonasschnelli.ch
OK seed.btc.petertodd.net (37 results)
OK seed.bitcoin.sprovoost.nl (36 results)
OK dnsseed.emzy.de (40 results)
OK see
...
💬 hebasto commented on pull request "depends: hard-code necessary c(xx)flags rather than setting them per-host":
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-2901416521)
My Guix build:
```
aarch64
7a5858e3364998675e06a2bec330a6fb9f619613a9228a568b5c98cba873460f guix-build-0275825f7ace/output/aarch64-linux-gnu/SHA256SUMS.part
a8b08f5f246ca97ae26bf9142355797f82d3f1e7eb2ffbc1b7d78f35c4c0cd4d guix-build-0275825f7ace/output/aarch64-linux-gnu/bitcoin-0275825f7ace-aarch64-linux-gnu-debug.tar.gz
92cbdec0549173a3e3a90ac81ad2dac98d9758a76efe12a6d205cc1265f9f701 guix-build-0275825f7ace/output/aarch64-linux-gnu/bitcoin-0275825f7ace-aarch64-linux-gnu.tar.gz
c7915b68
...
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-2901416521)
My Guix build:
```
aarch64
7a5858e3364998675e06a2bec330a6fb9f619613a9228a568b5c98cba873460f guix-build-0275825f7ace/output/aarch64-linux-gnu/SHA256SUMS.part
a8b08f5f246ca97ae26bf9142355797f82d3f1e7eb2ffbc1b7d78f35c4c0cd4d guix-build-0275825f7ace/output/aarch64-linux-gnu/bitcoin-0275825f7ace-aarch64-linux-gnu-debug.tar.gz
92cbdec0549173a3e3a90ac81ad2dac98d9758a76efe12a6d205cc1265f9f701 guix-build-0275825f7ace/output/aarch64-linux-gnu/bitcoin-0275825f7ace-aarch64-linux-gnu.tar.gz
c7915b68
...
🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2860445285)
Code review up to 412a9e0e69ccf461554660b7c622b768cfb8ccb3
Also did some fuzzing and mutate the new code added and tests, craches occur as expected.
> If a transaction is encountered whose addition would violate the limit, it is removed, together with all its descendants.
In real-world scenarios it is unlikely we encounter those transactions that will exceed the cluster count limit, however when it occur I assume that the newly connected block will have similar transaction with the reor
...
(https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2860445285)
Code review up to 412a9e0e69ccf461554660b7c622b768cfb8ccb3
Also did some fuzzing and mutate the new code added and tests, craches occur as expected.
> If a transaction is encountered whose addition would violate the limit, it is removed, together with all its descendants.
In real-world scenarios it is unlikely we encounter those transactions that will exceed the cluster count limit, however when it occur I assume that the newly connected block will have similar transaction with the reor
...
💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2102062249)
This method's name aligns with its functionality.
However with the addition of this commit I think there is ambiguity currently between the size oversize, the count oversize and both. This makes it a bit confusing to follow.
maybe separate the two.
oversize should be for size.
overlimit or something like it for both
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2102062249)
This method's name aligns with its functionality.
However with the addition of this commit I think there is ambiguity currently between the size oversize, the count oversize and both. This makes it a bit confusing to follow.
maybe separate the two.
oversize should be for size.
overlimit or something like it for both
💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2102457782)
assert that we trim when oversized ?
```suggestion
assert(!removed.empty());
auto removed_set = top_sim.MakeSet(removed);
```
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2102457782)
assert that we trim when oversized ?
```suggestion
assert(!removed.empty());
auto removed_set = top_sim.MakeSet(removed);
```