Conversation
|
Noting that this innocuous error does occur on the first start of the bundled node: |
Ah that's not unexpected; though we can fix that now. We currently have a loop of: loop {
let chain_tip = store.get_latest_header().await.block_num();
if let Ok(subscription) = block_producer.subscribe(chain_tip).await {
break subscription;
}
}which means there is a race condition between us fetching the chain tip and subscribing. @SantiagoPittella I think we now remove the
|
| "genesis block signature verification failed", | ||
| ); | ||
|
|
||
| Ok(Self(block)) |
There was a problem hiding this comment.
This will work for any valid signature<> key pair, which means the store will accept any genesis block as valid.
I wonder if we should instead keep the validator_key argument on the store bootstrap command - but change it to only mean the public key of the validator (AFAIU, currently a private key is provided).
E.g. validator_key -> validator_pk?
This way, the store is sure that the genesis block comes from the canonical validator.
There was a problem hiding this comment.
We'll need to think about how this should work though.
afaik currently the pub key for each block isn't derived from the config, but rather its the key specified in the genesis block. As in, by definition, the node cannot be configured with a key -- otherwise the node operator can determine the "valid" key.
There was a problem hiding this comment.
I wonder if we should instead keep the validator_key argument on the store bootstrap command - but change it to only mean the public key of the validator (AFAIU, currently a private key is provided).
E.g. validator_key -> validator_pk?
The store bootstrap command does not take a private key:
Usage: miden-node store bootstrap --data-directory <DIR> --genesis-block <FILE>
Options:
--data-directory <DIR> Directory in which to store the database and raw block data [env: MIDEN_NODE_DATA_DIRECTORY=]
--genesis-block <FILE> Path to the pre-signed genesis block file produced by the validator
-h, --help Print help (see more with '--help')
This will work for any valid signature<> key pair, which means the store will accept any genesis block as valid.
I think this is the point - that the node operator relies on the validator as the source of truth for this.
There was a problem hiding this comment.
@bobbinth the validator currently doesn't ensure the "canonical chain" at all. As in, the node operator could theoretically modify a historical block and the validator would still sign the subsequent ones (because the validator doesn't check the chain of commitments and signatures).
Is this something we want to add to the validator? Any of these:
- Track the chain tip
- Verify
prev_block_commitmentlinks to the last block it signed - Verify
block_numis sequential - Verify
chain_commitment(the MMR) is consistent - Verify the previous block's signature
There was a problem hiding this comment.
@bobbinth the validator currently doesn't ensure the "canonical chain" at all. As in, the node operator could theoretically modify a historical block and the validator would still sign the subsequent ones (because the validator doesn't check the chain of commitments and signatures).
Is this something we want to add to the validator?
I think the validator should make sure the block it is signing is the valid next block in the chain (for some reason, I thought that was already the case, but if not, let's create an issue for this).
The way it could work:
- We use the validator to "bootstrap" the genesis block (I think this is what this PR is about, but I haven't reviewed the details yet). This results in the validator keeping track of the chain tip (which is at first just the genesis block).
- For any new block that the validator needs to sign, the validator:
- Makes sure that all transactions in the block are the ones it has seen/validated before (we already do this, I believe).
- Makes sure that given this transactions, we get the block header that we need to sign (we already do this, I believe).
- Makes sure that the new block header is built on top of the previous block. This may also require checking consistency of account and nullifier tree updates (it may be possible to incorporate this in the above two checks, but I'm not 100% sure).
- If all looks good, the validator signs the block, and sets the new block's header as the chain tip.
| /// to disk. | ||
| async fn build_and_write_genesis( | ||
| config: GenesisConfig, | ||
| signer: impl BlockSigner, |
There was a problem hiding this comment.
Would suggest we use &dyn BlockSigner since this isn't really performance critical.
There was a problem hiding this comment.
Think it gets gnarly because of the associated error type
pub trait BlockSigner {
type Error: error::Error + Send + Sync + 'static;
Closes #1761
Tested via the following commands locally: