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

Add the logic that allows rotation of the PoA key #1901

Closed
xgreenx opened this issue May 18, 2024 · 1 comment · Fixed by #2086
Closed

Add the logic that allows rotation of the PoA key #1901

xgreenx opened this issue May 18, 2024 · 1 comment · Fixed by #2086

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented May 18, 2024

The PoA key is defined by the ChainConfig and currently supports only one value. We need to support the key's rotation. As an example, we could have a new variant with listed PoA key rotations.

pub enum ConsensusConfig {
    PoA {
        signing_key: Address,
    },
    RotatedPoA {
        /// The key of the map is the block height at which the rotation occurs.
        /// The value is the new signing key.
        rotations: BTreeMap<BlockHeight, Address>,
    },
}
@JoeruCodes
Copy link

JoeruCodes commented May 25, 2024

For the default_poa function for the RotatedPoA Variant the BTreeMap would be generated from this SecretKey::new_from_mnemonic_phrase_with_path(<PHRASE>, <PATH>) where PATH is offsetted by format!("m/44'/60'/0'/0/{}", nKeys) would be the correct approach to generate the keys right?

/// A default secret key to use for testing purposes only
pub fn default_consensus_dev_key() -> SecretKey {
    // Derived from:
    //  - Mnemonic phrase: "winner alley monkey elephant sun off boil hope toward boss bronze dish"
    //  - Path: "m/44'/60'/0'/0/0"
    // Equivalent to:
    //  `SecretKey::new_from_mnemonic_phrase_with_path(..)`
    let bytes: [u8; 32] = [
        0xfb, 0xe4, 0x91, 0x78, 0xda, 0xc2, 0xdf, 0x5f, 0xde, 0xa7, 0x4a, 0x11, 0xa9,
        0x0f, 0x99, 0x77, 0x62, 0x5f, 0xe0, 0x23, 0xcd, 0xf6, 0x41, 0x4b, 0xfd, 0x63,
        0x9d, 0x32, 0x7a, 0x2e, 0x9d, 0xdb,
    ];
    SecretKey::try_from(bytes.as_slice()).expect("valid key")
}

The primary PoA is generated from these PATHs and PHRASE

EDIT:
I just realised these are dev_keys... So another likely approach could be is to hash the keys received from the Secret::default_consensus_dev_key() something like this (with a nonce counter or something) and then insert it into the BTree or AVL Tree...

impl SecretKey{
    // other methods ....

    pub fn rotate(&self, nonce: u32) -> Self {
        let mut hasher = Sha256::new();
        hasher.update(self.as_ref());
        hasher.update(&nonce.to_le_bytes());
        let result = hasher.finalize();
        let mut new_key = [0u8; Self::LEN];
        new_key.copy_from_slice(&result[0..Self::LEN]);
        SecretKey(Bytes32::from(new_key))
    }
}

@xgreenx xgreenx self-assigned this Jun 10, 2024
@xgreenx xgreenx removed their assignment Aug 14, 2024
GoldenPath1109 added a commit to GoldenPath1109/fuel-core that referenced this issue Sep 7, 2024
Closes FuelLabs/fuel-core#1901
Closes FuelLabs/fuel-core#1919

The `ConsensusConfig` now contains one more variant, `PoAV2`, that also
overrides the PoA at specific block heights. It means that starting this
block the node expects blocks signed by another PoA key.

Along with that, I've added additional features to automate the
migration of the state so it is compatible with the new chain
configuration:
- During start-up, the `fuel-core` binary checks if there are any
changes in the chain config. If they are, the binary re-inserts the new
`Conesnsus::Genesis` type in the `SealedBlockConsensus` table. Now we
can do FuelLabs/chain-configuration#1.
- The `fuel-core` to react to `signing_key_overrides`. If the blocks at
the corresponding overwritten block height have different signers, the
`fuel-core` rollbacks to the height before overriding occurs.

Side changes:
- Renamed the methods of the `FuelService` to be more explicit on the
logic.
- Moved business logic from `FuelService::Task` to `FuelService`.

## Checklist
- [x] New behavior is reflected in tests

### Before requesting review
- [x] I have reviewed the code myself
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants