📝 romanz opened a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657)
It will allow fetching specific transactions using an external index, following https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3267485313.
Currently, electrs and other indexers map between an address/scripthash to the list of the relevant transactions.
However, in order to fetch those transactions from bitcoind, electrs relies on reading the whole block and post-filtering for a specific transaction[^1]. Other indexers use a `txindex` to fetch a transaction using its txid [^2][^
...
(https://github.com/bitcoin/bitcoin/pull/33657)
It will allow fetching specific transactions using an external index, following https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3267485313.
Currently, electrs and other indexers map between an address/scripthash to the list of the relevant transactions.
However, in order to fetch those transactions from bitcoind, electrs relies on reading the whole block and post-filtering for a specific transaction[^1]. Other indexers use a `txindex` to fetch a transaction using its txid [^2][^
...
📝 ubbabeck opened a pull request: "test: multisig verify spend from 100 of 999 taproot multisig wallet"
(https://github.com/bitcoin/bitcoin/pull/33658)
- **miniscript: fixes #29098 by only use first k valid signatures #31719**
- ** test: verify spend from 999-of-999 taproot multisig wallet #28212**
Testing of https://github.com/bitcoin/bitcoin/pull/31719 to test if it is a proper candidate ref https://github.com/bitcoin/bitcoin/pull/28212#issuecomment-2608930756.
Seems to help on my computer the test takes `build/test/functional/wallet_taproot.py 33.31s user 1.08s system 97% cpu 35.114 total`
to complete.
feel free to close the pr
...
(https://github.com/bitcoin/bitcoin/pull/33658)
- **miniscript: fixes #29098 by only use first k valid signatures #31719**
- ** test: verify spend from 999-of-999 taproot multisig wallet #28212**
Testing of https://github.com/bitcoin/bitcoin/pull/31719 to test if it is a proper candidate ref https://github.com/bitcoin/bitcoin/pull/28212#issuecomment-2608930756.
Seems to help on my computer the test takes `build/test/functional/wallet_taproot.py 33.31s user 1.08s system 97% cpu 35.114 total`
to complete.
feel free to close the pr
...
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443315198)
This test is further updated in a later commit in this PR -- please let me know what you think of the state it ends up in and if it could use further improvements.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443315198)
This test is further updated in a later commit in this PR -- please let me know what you think of the state it ends up in and if it could use further improvements.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443316513)
Seems redundant, given that we have an `EXCLUSIVE_LOCKS_REQUIRED` annotation on `GetChangeSet()` already? Will leave this as-is.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443316513)
Seems redundant, given that we have an `EXCLUSIVE_LOCKS_REQUIRED` annotation on `GetChangeSet()` already? Will leave this as-is.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443318588)
`clear()` doesn't change the capacity of the vector, so there's no need to `reserve()` again.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443318588)
`clear()` doesn't change the capacity of the vector, so there's no need to `reserve()` again.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443321045)
This is a reference to Rule #5 in `doc/policy/mempool-replacements.md`.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443321045)
This is a reference to Rule #5 in `doc/policy/mempool-replacements.md`.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443329464)
I think "spent" would be confusing, as the purpose of this object is to store the data needed to undo the removal of a utxo from the unspent_prevouts list, in the event that a transaction spending the prevout fails to get into the mempool.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443329464)
I think "spent" would be confusing, as the purpose of this object is to store the data needed to undo the removal of a utxo from the unspent_prevouts list, in the event that a transaction spending the prevout fails to get into the mempool.
💬 Raimo33 commented on pull request "bench: replace embedded raw block with configurable block generator":
(https://github.com/bitcoin/bitcoin/pull/32554#issuecomment-3419716580)
Concept ACK for more realistic benchmarks
(https://github.com/bitcoin/bitcoin/pull/32554#issuecomment-3419716580)
Concept ACK for more realistic benchmarks
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443345859)
Not sure I follow the reasoning here -- an RBF that fails due to cluster size limits might succeed if a child is added that adds additional conflicts, thereby making space in the affected cluster.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2443345859)
Not sure I follow the reasoning here -- an RBF that fails due to cluster size limits might succeed if a child is added that adds additional conflicts, thereby making space in the affected cluster.
🚀 fanquake merged a pull request: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33625)
(https://github.com/bitcoin/bitcoin/pull/33625)
✅ romanz closed a pull request: "index: store per-block transaction locations for efficient lookups"
(https://github.com/bitcoin/bitcoin/pull/32541)
(https://github.com/bitcoin/bitcoin/pull/32541)
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3419729543)
Closing this PR, the "index-less" approach will be implemented in https://github.com/bitcoin/bitcoin/pull/33657.
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3419729543)
Closing this PR, the "index-less" approach will be implemented in https://github.com/bitcoin/bitcoin/pull/33657.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3419748327)
Updated to use `std::barrier` for the completion synchronization instead of acquiring a semaphore for each thread, as suggested by @l0rinc .
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3419748327)
Updated to use `std::barrier` for the completion synchronization instead of acquiring a semaphore for each thread, as suggested by @l0rinc .
🤔 stringintech reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3353593203)
I have started a per-commit review. These are mostly related to the first commit (dc504f57) + CI comments submitted earlier.
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3353593203)
I have started a per-commit review. These are mostly related to the first commit (dc504f57) + CI comments submitted earlier.
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443264406)
Isn't it better to set `status` to `SCRIPT_VERIFY_OK` internally (if not null) after above checks rather than expecting callers to pre-initialize it? (also adapting the doc in the header for `btck_script_pubkey_verify()` and removing the two lines that revert the status value to `OK` in `run_verify_test()`)
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443264406)
Isn't it better to set `status` to `SCRIPT_VERIFY_OK` internally (if not null) after above checks rather than expecting callers to pre-initialize it? (also adapting the doc in the header for `btck_script_pubkey_verify()` and removing the two lines that revert the status value to `OK` in `run_verify_test()`)
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443282691)
Duplicate declarations?
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443282691)
Duplicate declarations?
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443266860)
Why we are skipping value `1`?
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443266860)
Why we are skipping value `1`?
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443366124)
A bit confused by this part.
For example for the first assertion, I guess we wanted to compare `object2.get() != distinct_object.get()` to verify the copy constructor worked? And maybe for the second assertion we wanna check if `object2` and `distinct_object` have the same bytes size?
Also for the `Copy assignment` section assertions that comes after.
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2443366124)
A bit confused by this part.
For example for the first assertion, I guess we wanted to compare `object2.get() != distinct_object.get()` to verify the copy constructor worked? And maybe for the second assertion we wanna check if `object2` and `distinct_object` have the same bytes size?
Also for the `Copy assignment` section assertions that comes after.
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2442427401)
Missing `BITCOINKERNEL_WARN_UNUSED_RESULT`.
Also for:
- `btck_transaction_out_point_get_txid()`
- `btck_chain_contains()`
- `btck_transaction_out_point_get_index()`
- `btck_txid_equals()`
- `btck_block_hash_equals()`
Not sure about the following (they return 0 on success):
- `*_to_bytes()`
- `btck_chainstate_manager_options_set_wipe_dbs()`
- `btck_chainstate_manager_import_blocks()`
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2442427401)
Missing `BITCOINKERNEL_WARN_UNUSED_RESULT`.
Also for:
- `btck_transaction_out_point_get_txid()`
- `btck_chain_contains()`
- `btck_transaction_out_point_get_index()`
- `btck_txid_equals()`
- `btck_block_hash_equals()`
Not sure about the following (they return 0 on success):
- `*_to_bytes()`
- `btck_chainstate_manager_options_set_wipe_dbs()`
- `btck_chainstate_manager_import_blocks()`
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3419973259)
Some of the PR description is reused from the previous PR, but is now stale.
> This PR is adding support for using the transaction's position within its block to be able to fetch it directly using REST API, using the following HTTP request (to fetch the N-th transaction from BLOCKHASH):
This does not support fetching an N-th transaction, only a slice of a block.
> If binary response format is used, the transaction data will be read directly from the storage and sent back to the client, withou
...
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3419973259)
Some of the PR description is reused from the previous PR, but is now stale.
> This PR is adding support for using the transaction's position within its block to be able to fetch it directly using REST API, using the following HTTP request (to fetch the N-th transaction from BLOCKHASH):
This does not support fetching an N-th transaction, only a slice of a block.
> If binary response format is used, the transaction data will be read directly from the storage and sent back to the client, withou
...