Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A chain with single validator could be increased even I set an INCORRECT bls_pub_key. #1509

Closed
yangby-cryptape opened this issue Oct 27, 2023 · 5 comments
Assignees
Labels
document enhancement New feature or request

Comments

@yangby-cryptape
Copy link
Collaborator

yangby-cryptape commented Oct 27, 2023

Description

I have made a mistake:

So, the bls_privkey_file didn't match any keys in metadata.
The chain should be an observer, its height shouldn't be increased.

But, in fact, the chain is running as a normal chain.

The only issue is that no observers are able to connect to this node.

Reproduce

I only changes the file devtools/chain/specs/single_node/chain-spec.toml:

bls_pub_key = "0xa26e3fe1cf51bd4822072c61bdc315ac32e3d3c2e2484bb92942666399e863b4bf56cf2926383cc706ffc15dfebc85c6"

with the following patch:

- bls_pub_key = "0xa26e3fe1cf51bd4822072c61bdc315ac32e3d3c2e2484bb92942666399e863b4bf56cf2926383cc706ffc15dfebc85c6"
+ bls_pub_key = "0xac85bbb40347b6e06ac2dc2da1f75eece029cdc0ed2d456c457d27e288bfbfbcd4c5c19716e9b250134a0e76ce50fa22"

And the chain works well in CI.

@yangby-cryptape yangby-cryptape changed the title [Why] A chain with single validator could be increased even I set an INCORRECT bls_pub_key. [Bug] A chain with single validator could be increased even I set an INCORRECT bls_pub_key. Oct 27, 2023
@yangby-cryptape yangby-cryptape changed the title [Bug] A chain with single validator could be increased even I set an INCORRECT bls_pub_key. [Bug?] A chain with single validator could be increased even I set an INCORRECT bls_pub_key. Oct 27, 2023
@Flouse Flouse added the agenda label Oct 27, 2023
@yangby-cryptape yangby-cryptape changed the title [Bug?] A chain with single validator could be increased even I set an INCORRECT bls_pub_key. A chain with single validator could be increased even I set an INCORRECT bls_pub_key. Nov 24, 2023
@yangby-cryptape yangby-cryptape added the d:confirmation Discussion required to confirm whether it's a bug label Nov 24, 2023
@Flouse
Copy link
Contributor

Flouse commented Nov 24, 2023

@driftluo can you confirm that if this is a bug?

@driftluo
Copy link
Contributor

I personally think the problem lies here. If the proposal is made by oneself, proof verification is not performed:
@KaoImin

// If the block is proposed by self, it does not need to check. Get full signed
// transactions directly.
if !exemption {
if let Err(e) = self.inner_check_block(ctx.clone(), &proposal).await {
let mut reason = self.last_check_block_fail_reason.write();
*reason = e.to_string();
return Err(e.into());
}
}

previous_block.header.timestamp,
) {
return Err(ProtocolError::from(ConsensusError::InvalidTimestamp));
}
self.adapter
.verify_proof(
ctx.clone(),
previous_block.clone(),
proposal.proof.clone(),
)

@KaoImin
Copy link
Contributor

KaoImin commented Nov 29, 2023

I personally think the problem lies here. If the proposal is made by oneself, proof verification is not performed: @KaoImin

// If the block is proposed by self, it does not need to check. Get full signed
// transactions directly.
if !exemption {
if let Err(e) = self.inner_check_block(ctx.clone(), &proposal).await {
let mut reason = self.last_check_block_fail_reason.write();
*reason = e.to_string();
return Err(e.into());
}
}

previous_block.header.timestamp,
) {
return Err(ProtocolError::from(ConsensusError::InvalidTimestamp));
}
self.adapter
.verify_proof(
ctx.clone(),
previous_block.clone(),
proposal.proof.clone(),
)

I suggest check the bls key is correct when init.

@Flouse Flouse added t:bug Something isn't working and removed d:confirmation Discussion required to confirm whether it's a bug labels Nov 29, 2023
@Flouse Flouse closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2023
@Flouse
Copy link
Contributor

Flouse commented Dec 6, 2023

For the single Axon node mode

@driftluo has explained: If the proposal is made by oneself, proof verification is not performed.

This mode could be ignored because in a paractical chain, the single node mode won't be used.
Just be sure to setup a right key in this mode. I think a better key-generator tool will help.

For the multi Axon nodes mode

If one of the bls_pub_keys in chain-spec.toml is wrong, then verify_proof function will failed, and the error message [consensus] verify_proof_signature error will be displayed.

@Flouse
Copy link
Contributor

Flouse commented Dec 6, 2023

@KaoImin suggested that checking the bls key is correct when init.

But during the init step, the program doesn't know if the bls_privkey_file in config.toml is associated with a validator, so it can't just check if the bls_pub_key is the configured bls_privkey's pub key.

@Flouse Flouse added document enhancement New feature or request and removed t:bug Something isn't working labels Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants