Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ“ aguycalled opened a pull request: "Create wallet from seed"
(https://github.com/bitcoin/bitcoin/pull/30606)
This PR adds the option to create a wallet from a seed. The seed of a wallet can be obtained using the RPC command `getblsctseed`.

Two options to create a wallet:
- Using the `navio-wallet` tool together with the option `seed`. Example:
`navio-wallet -blsct -chain=blsctregtest -wallet=wallet_name -seed="71187dc33c03914c630e87b205777d51f7bd2749bc1164795e0134544d43d9ee" create`
- Using the rpc command `createwallet`. Example:
`navio-cli --blsctregtest -named createwallet wallet_name=walle
...
πŸ’¬ fjahr commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1707958169)
See the description: "The first commit shows how to reproduce the error shown in https://github.com/bitcoin/bitcoin/issues/30514 and how it could be fixed with a minimal change. The second commit adds an explicit sanitizer that will probably be the preferred approach. I can squash the commits but first I would like to hear conceptual feedback since https://github.com/bitcoin/bitcoin/issues/30514 suggested to remove the block height instead."
πŸ‘ maflcko approved a pull request: "node: reduce unsafe uint256S usage"
(https://github.com/bitcoin/bitcoin/pull/30569#pullrequestreview-2226246193)
left a nit. Feel free to ignore.

Otherwise this looks good on a first glance.
πŸ’¬ maflcko commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1707958805)
If you re-touch this commit, it could make sense to remove the `__func__` printing in the log call below. It doesn't add any value and may be duplicate. I understand it is unrelated, but when touching a test-only function, may as well fix all trivial issues in one go. (See the commit I referred to in https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1703942199, which also has steps to reproduce and test in the description)
πŸ’¬ fjahr commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1707962074)
We would probably have to cast somewhere else then because `nHeight` is `int`.
βœ… maflcko closed a pull request: "Create wallet from seed"
(https://github.com/bitcoin/bitcoin/pull/30606)
πŸ’¬ fjahr commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#issuecomment-2274401173)
> Just to clarify, I am happy to review and ack this approach here, but I haven't acked the current state of the pull request, because it leaves bugs unfixed

Thanks, that was not clear to me indeed.

> I haven't tried it, but https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1700350964 (the error message for the user will not be: "Corrupt metadata, block height corrupt", but instead "A forked headers-chain with more work than the chain with the snapshot base block header exists. Ple
...
πŸ“ theStack opened a pull request: "contrib: support reading XORed blocks in linearize-data.py script"
(https://github.com/bitcoin/bitcoin/pull/30607)
This PR is a small follow-up for #28052, adding support for the block linearization script to handle XORed blocksdir *.dat files. Note that if no xor.dat file exists, the XOR pattern is set to all-zeros, in order to still support blockdirs that have been created with versions earlier than 28.x.

Partly fixes issue #30599.
πŸ’¬ maflcko commented on pull request "test: [refactor] Use g_rng/m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#issuecomment-2274410714)
Dropped one commit, because a conflicting pull has already picked it up.
πŸ’¬ fjahr commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#issuecomment-2274410734)
A little recap so far: While I still don’t find the arguments *against* having the height included convincing I am currently leaning towards going with the approach in #30598 and removing it for now. My reasoning is as follows:

- While the arguments against may not be strong I conceive that it would be better if we had a clear plan for how we use the height which would give a stronger argument for keeping it in. We can re-add the height in a new version together with a clear plan of how it is
...
πŸ’¬ fjahr commented on pull request "Assumeutxo: Sanitize block height in metadata":
(https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1707978449)
So I was still hunting for the conceptual consensus and didn't change this yet. I think we are coming to an end though (see my comment below). It would be great if you could indicate where you stand on this though @ismaelsadeeq . If more support for the approach here I will squash the commits and implement the suggested improvements.
πŸ’¬ theuni commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2274440199)
Very cool. Can't wait to dig in when I have some free time.
πŸ€” hodlinator reviewed a pull request: "Add a "tx output spender" index"
(https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-2219808525)
Concept ACK 40e44e96c38593210877f0b5d062e71a8dd292a5

Can see how this is useful for L2s. Is a variant of this PR known to be used in production?

I do agree with @Pantamis in https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-1320276112 that having it take 3x the space of **TxIndex** seems unfortunate, would prefer it used `CDiskTxPos` for the values.


Commits could be squashed/rewritten. Seems like the latter two are responses to PR feedback rather than "telling a story".
Prev
...
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1704599014)
nit: Inconsistent style, should probably have initializer list on own line in both cases.
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1706479714)
Suggestion: use `optional` to promote error checking or `[[nodiscard]] bool` and document non-const "out"-references by variable naming or comment.

```suggestion
std::optional<Txid> TxoSpenderIndex::FindSpender(const COutPoint& txo) const
{
uint256 tx_hash_out;
if (m_db->Read(std::pair{DB_TXOSPENDERINDEX, txo}, tx_hash_out))
return Txid::FromUint256(tx_hash_out);
return std::nullopt;
}
```
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1706495582)
No longer used?
```suggestion
#include <common/args.h>
```
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1706507458)
Could use `pair{}` for brevity here and elsewhere (https://stackoverflow.com/a/41521422).
```suggestion
batch.Write(std::pair{DB_TXOSPENDERINDEX, outpoint}, hash);
```
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1706465407)
nit: Consider using `default` syntax to indicate that you only included this in the **.cpp** file so that `m_db` can be destructed:
```suggestion
TxoSpenderIndex::~TxoSpenderIndex() = default;
```
`TxIndex` already does this.

(Inspired by https://github.com/bitcoin/bitcoin/pull/19988#discussion_r494257310).
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1706536603)
Suggestion: Clarify the conditional branches and state of `mempool_only` later in the code:
```suggestion
// do nothing if only mempool querying is allowed
```
πŸ’¬ hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1706511696)
Superfluous comment copied from **txindex.h**.
```suggestion
```