💬 tdb3 commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1631217975)
The `exists()` asserts are a nice improvement/sanity check.
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1631217975)
The `exists()` asserts are a nice improvement/sanity check.
💬 tdb3 commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631249895)
As a follow-up, checked that bitcoind does actually send the exact reason phrase "No Content", so the updates made are correct.
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631249895)
As a follow-up, checked that bitcoind does actually send the exact reason phrase "No Content", so the updates made are correct.
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154903353)
All the above should be fixed now, except for `g_best_block_cv.wait_until` (but that's not blocking).
The biggest change is in 914c26bbd4d7e87af4e0d0edf9af69ae76668b7e. It now makes the `options` argument mandatory for `BlockAssembler` constructor, dropping implicit handling of `ArgsManager`. The caller i.e. the `Mining` interface implementation now handles this.
In Stratum v2 the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This ne
...
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154903353)
All the above should be fixed now, except for `g_best_block_cv.wait_until` (but that's not blocking).
The biggest change is in 914c26bbd4d7e87af4e0d0edf9af69ae76668b7e. It now makes the `options` argument mandatory for `BlockAssembler` constructor, dropping implicit handling of `ArgsManager`. The caller i.e. the `Mining` interface implementation now handles this.
In Stratum v2 the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This ne
...
💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271509)
done in first commit now
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271509)
done in first commit now
💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271559)
done
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631271559)
done
💬 sdaftuar commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#issuecomment-2154918042)
utACK 30a01134c
(https://github.com/bitcoin/bitcoin/pull/29496#issuecomment-2154918042)
utACK 30a01134c
💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#issuecomment-2154934386)
rebased due to silent merge conflict, with cleanups suggested by @glozow :+1:
(https://github.com/bitcoin/bitcoin/pull/30082#issuecomment-2154934386)
rebased due to silent merge conflict, with cleanups suggested by @glozow :+1:
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154948888)
You can see the interface in action in https://github.com/bitcoin/bitcoin/pull/29432/commits/1618c3dd86d65cf00f5f3c2c30c1db2b0e9dd9ca
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2154948888)
You can see the interface in action in https://github.com/bitcoin/bitcoin/pull/29432/commits/1618c3dd86d65cf00f5f3c2c30c1db2b0e9dd9ca
👋 Sjors's pull request is ready for review: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200)
(https://github.com/bitcoin/bitcoin/pull/30200)
⚠️ maflcko opened an issue: "fuzz: wallet_bdb_parser: implicit-signed-integer-truncation wallet/migrate.cpp:554:35 "
(https://github.com/bitcoin/bitcoin/issues/30247)
Full error:
```
wallet/migrate.cpp:554:35: runtime error: implicit conversion from type 'int64_t' (aka 'long') of value -1 (64-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
```
The code:
```cpp
uint32_t expected_last_page = (size / page_size) - 1;
```
This may lead to a fuzz runtime error when `(size / page_size)` is `0`. I don't think this can lead to issues for real users, because it can only happen with corrupt files an
...
(https://github.com/bitcoin/bitcoin/issues/30247)
Full error:
```
wallet/migrate.cpp:554:35: runtime error: implicit conversion from type 'int64_t' (aka 'long') of value -1 (64-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
```
The code:
```cpp
uint32_t expected_last_page = (size / page_size) - 1;
```
This may lead to a fuzz runtime error when `(size / page_size)` is `0`. I don't think this can lead to issues for real users, because it can only happen with corrupt files an
...
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2154958567)
Updated to use the interface proposed in #30200. This also fixes a small bug: `-blockmintxfee` and `-blockmaxweight` are no longer ignored. When the latter argument is unset, by default blocks have a `4000` byte safety margin, just like with `getblocktemplate`.
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2154958567)
Updated to use the interface proposed in #30200. This also fixes a small bug: `-blockmintxfee` and `-blockmaxweight` are no longer ignored. When the latter argument is unset, by default blocks have a `4000` byte safety margin, just like with `getblocktemplate`.
💬 maflcko commented on issue "fuzz: wallet_bdb_parser: implicit-signed-integer-truncation wallet/migrate.cpp:554:35 ":
(https://github.com/bitcoin/bitcoin/issues/30247#issuecomment-2154959411)
Found by @murchandamus in https://github.com/bitcoin-core/qa-assets/pull/186#issuecomment-2154348655
```
Run wallet_bdb_parser with args ['/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/wallet_bdb_parser')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1090373666
INFO: Loaded 1 modules (584206 inline 8-bit counters): 584206 [0x55fc8b5c2ec8, 0x55fc8b651
...
(https://github.com/bitcoin/bitcoin/issues/30247#issuecomment-2154959411)
Found by @murchandamus in https://github.com/bitcoin-core/qa-assets/pull/186#issuecomment-2154348655
```
Run wallet_bdb_parser with args ['/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/wallet_bdb_parser')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1090373666
INFO: Loaded 1 modules (584206 inline 8-bit counters): 584206 [0x55fc8b5c2ec8, 0x55fc8b651
...
💬 fanquake commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631315946)
No objections from me.
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631315946)
No objections from me.
🤔 glozow reviewed a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2104768021)
light review ACK 39d135e79f3f0c40dfd8fad2c53723d533cd19b4
Would be nice to have this work for `create_self_transfer_multi` as well
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2104768021)
light review ACK 39d135e79f3f0c40dfd8fad2c53723d533cd19b4
Would be nice to have this work for `create_self_transfer_multi` as well
💬 theStack commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#issuecomment-2154998767)
One drawback of this change is that a user doesn't see download progress per file any more (relevant if e.g. servers are slow or the connection is bad for any other reason; for me this currently takes several minutes per file :/) and hence is left in the dark whether there is still activity, so Concept ~0.
(https://github.com/bitcoin/bitcoin/pull/29970#issuecomment-2154998767)
One drawback of this change is that a user doesn't see download progress per file any more (relevant if e.g. servers are slow or the connection is bad for any other reason; for me this currently takes several minutes per file :/) and hence is left in the dark whether there is still activity, so Concept ~0.
💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631335859)
okie doke, added
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1631335859)
okie doke, added
🤔 glozow reviewed a pull request: "test: expand LimitOrphan and EraseForPeer coverage"
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2104789432)
almost ack 6831f3972ee5d660ce0bff9bb573745338417544, thanks for taking suggestions
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2104789432)
almost ack 6831f3972ee5d660ce0bff9bb573745338417544, thanks for taking suggestions
💬 glozow commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631339561)
`EraseOrphansFor` doesn't exist
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631339561)
`EraseOrphansFor` doesn't exist
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2155027685)
The `generateblock` block RPC relies on not passing the mempool to `BlockAssembler`, in order to allow manual transaction selection. So I added `bool use_mempool = true` to `createNewBlock`.
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2155027685)
The `generateblock` block RPC relies on not passing the mempool to `BlockAssembler`, in order to allow manual transaction selection. So I added `bool use_mempool = true` to `createNewBlock`.
💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631356425)
:sweat: fixed
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1631356425)
:sweat: fixed