๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149709427)
Since Iโve removed the special casing that makes us bail if we start with 500 in the first place, I have left it at the start of the loop.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149709427)
Since Iโve removed the special casing that makes us bail if we start with 500 in the first place, I have left it at the start of the loop.
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149709631)
Iโve adopted your suggestion, thanks
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149709631)
Iโve adopted your suggestion, thanks
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149712008)
Right, if txids is empty, we already return an empty vector anyway, and the 500 limit is already checked at the start of the loop. Sorry, @furszy, I changed my mind about [your prior suggestion to add an early exit](https://github.com/bitcoin/bitcoin/pull/27021/files#diff-c065d4cd2398ad0dbcef393c5dfc53f465bf44723348892395fffd2fb3bac522).
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149712008)
Right, if txids is empty, we already return an empty vector anyway, and the 500 limit is already checked at the start of the loop. Sorry, @furszy, I changed my mind about [your prior suggestion to add an early exit](https://github.com/bitcoin/bitcoin/pull/27021/files#diff-c065d4cd2398ad0dbcef393c5dfc53f465bf44723348892395fffd2fb3bac522).
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149422663)
See response on prior comment
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149422663)
See response on prior comment
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149659725)
Removed unimplemented function
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149659725)
Removed unimplemented function
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149445980)
Yeah, it felt safer to first update everything, then to start deleting things. I did try combining the loops and it did not break anything, though. Still deciding, maybe this could be a follow-up.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149445980)
Yeah, it felt safer to first update everything, then to start deleting things. I did try combining the loops and it did not break anything, though. Still deciding, maybe this could be a follow-up.
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149399579)
I am not sure I follow. For historical reasons, transactions are included in their own ancestor set and their own descendant set. So while we are iterating over descendants, we could be evaluating a transaction itself that has no ancestors left after they were picked into our โvirtual blockโ. If the evaluated transaction doesnโt have ancestors left, and we get its size with ancestors, we are comparing its own size with its own size in this inequality. Therefore, same size must be permitted.
I
...
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149399579)
I am not sure I follow. For historical reasons, transactions are included in their own ancestor set and their own descendant set. So while we are iterating over descendants, we could be evaluating a transaction itself that has no ancestors left after they were picked into our โvirtual blockโ. If the evaluated transaction doesnโt have ancestors left, and we get its size with ancestors, we are comparing its own size with its own size in this inequality. Therefore, same size must be permitted.
I
...
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149653633)
Yeah, agreed. Gonna remove it
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149653633)
Yeah, agreed. Gonna remove it
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149782804)
Iโve renamed the variable and better explained whatโs going on here.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149782804)
Iโve renamed the variable and better explained whatโs going on here.
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149731957)
Iโve removed `all_entries`. Thanks for catching that
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149731957)
Iโve removed `all_entries`. Thanks for catching that
๐ฌ willcl-ark commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1149825850)
I also do not get the error with:
```fish
โฟ ./src/bitcoind -datadir=/tmp/bitcoin_27302/ -conf=/tmp/bitcoin_27302/bitcoin.conf -rpcport=18555 -port=18556
```
But I do see the error (as described in the commit bullet point 2) with:
```fish
โฟ ./src/bitcoind -conf=/tmp/bitcoin_27302/bitcoin.conf -rpcport=18555 -port=18556
```
In my opinion:
> Show an error on startup if a bitcoin datadir that is being used contains a
> `bitcoin.conf` file that is ignored. There are two cases wher
...
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1149825850)
I also do not get the error with:
```fish
โฟ ./src/bitcoind -datadir=/tmp/bitcoin_27302/ -conf=/tmp/bitcoin_27302/bitcoin.conf -rpcport=18555 -port=18556
```
But I do see the error (as described in the commit bullet point 2) with:
```fish
โฟ ./src/bitcoind -conf=/tmp/bitcoin_27302/bitcoin.conf -rpcport=18555 -port=18556
```
In my opinion:
> Show an error on startup if a bitcoin datadir that is being used contains a
> `bitcoin.conf` file that is ignored. There are two cases wher
...
๐ ismaelsadeeq opened a pull request: "refactor: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).
Instead of using RPC calls
...
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).
Instead of using RPC calls
...
๐ฌ ismaelsadeeq commented on pull request "refactor: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1485964435)
@josibake @theStack @willcl-ark this is PR https://github.com/bitcoin/bitcoin/pull/27269 follow up, I will appreciate your reviews.
Thank you
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1485964435)
@josibake @theStack @willcl-ark this is PR https://github.com/bitcoin/bitcoin/pull/27269 follow up, I will appreciate your reviews.
Thank you
๐ฌ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1485975148)
> Left few more comments.
>
> Plus, while was checking the 1p1c test, couldn't resist myself and made it more friendly [furszy@5dc4b89](https://github.com/furszy/bitcoin-core/commit/5dc4b8957c7a65746666cb5f3e7bd6270b21f8c2). Feel free to take it if you like it.
Sorry, I was just going through the commits one by one and only rediscovered this suggestion this evening. It looks like there is some good stuff there. Iโll aim to incorporate it shortly, but it might not be tonight.
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1485975148)
> Left few more comments.
>
> Plus, while was checking the 1p1c test, couldn't resist myself and made it more friendly [furszy@5dc4b89](https://github.com/furszy/bitcoin-core/commit/5dc4b8957c7a65746666cb5f3e7bd6270b21f8c2). Feel free to take it if you like it.
Sorry, I was just going through the commits one by one and only rediscovered this suggestion this evening. It looks like there is some good stuff there. Iโll aim to incorporate it shortly, but it might not be tonight.
๐ theStack opened a pull request: "test: refactor: dedup mempool_package_limits.py subtests via decorator"
(https://github.com/bitcoin/bitcoin/pull/27350)
The subtests in the functional test mempool_package_limits.py all follow the same pattern:
1. first, check that the mempool is currently empty
2. create and submit certain single txs to the mempool, prepare a to-be-submitted package
3. check that submitting the package fails with a "package-mempool-limits" error on each tx result
4. after mining a block, check that re-submitting the package succeeds
Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so th
...
(https://github.com/bitcoin/bitcoin/pull/27350)
The subtests in the functional test mempool_package_limits.py all follow the same pattern:
1. first, check that the mempool is currently empty
2. create and submit certain single txs to the mempool, prepare a to-be-submitted package
3. check that submitting the package fails with a "package-mempool-limits" error on each tx result
4. after mining a block, check that re-submitting the package succeeds
Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so th
...
๐ theStack approved a pull request: "net: #27257 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/27324)
ACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
(https://github.com/bitcoin/bitcoin/pull/27324)
ACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
๐ ismaelsadeeq converted_to_draft a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).
Instead of using RPC calls
...
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).
Instead of using RPC calls
...
๐ ismaelsadeeq's pull request is ready for review: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
(https://github.com/bitcoin/bitcoin/pull/27349)
๐ ismaelsadeeq converted_to_draft a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).
Instead of using RPC calls
...
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).
Instead of using RPC calls
...
๐ apoelstra opened a pull request: "wallet: add `seeds` argument to `importdescriptors`"
(https://github.com/bitcoin/bitcoin/pull/27351)
This PR introduces the ability to import BIP32 master seeds alongside xpubs. It currently expects seeds to be provided in [BIP 93/codex32](https://github.com/bitcoin/bips/blob/master/bip-0093.mediawiki) either as a single seed or as a list of shares which can be assembled via Shamir Secret Sharing.
It could be generalized to other seed formats, e.g. a hex-encoding or the old `sethdseeds` WIF format, easily enough by just attempting to parse the input in those formats and retrying until we fin
...
(https://github.com/bitcoin/bitcoin/pull/27351)
This PR introduces the ability to import BIP32 master seeds alongside xpubs. It currently expects seeds to be provided in [BIP 93/codex32](https://github.com/bitcoin/bips/blob/master/bip-0093.mediawiki) either as a single seed or as a list of shares which can be assembled via Shamir Secret Sharing.
It could be generalized to other seed formats, e.g. a hex-encoding or the old `sethdseeds` WIF format, easily enough by just attempting to parse the input in those formats and retrying until we fin
...