Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

FM-254: Create checkpoint #273

Merged
merged 17 commits into from
Sep 26, 2023
Merged

FM-254: Create checkpoint #273

merged 17 commits into from
Sep 26, 2023

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Sep 19, 2023

Closes consensus-shipyard/ipc#303
Depends on consensus-shipyard/ipc-solidity-actors#202

Changes:

  • Check if the gateway is installed
  • Detect checkpoint period boundary
  • Fetch the current validator set
  • Create a Merkle tree out of the validator set
  • Construct checkpoint
  • Insert checkpoint into the ledger

I created a support data structure called ValidatorMerkleTree using the library we found. Had to fork it to update it to the latest ethers. There are many things we could change in that library to avoid the multitude of panic conditions. It only works for the happy path, but for now that is all we need. It's also a bit annoying that it converts resulting bytes into strings that I have to parse back bytes, but we can deal with these later.

ABI bindings built with:

export IPC_ACTORS_TAG=origin/fendermint247
make clean-ipc-actors-abi ipc-actors-abi

@aakoshh aakoshh marked this pull request as ready for review September 21, 2023 12:21
@aakoshh aakoshh force-pushed the fm-254-create-checkpoint branch from 07f6e4f to d9f7c8e Compare September 21, 2023 14:50
@aakoshh aakoshh mentioned this pull request Sep 26, 2023
Copy link
Contributor

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I still have a few blind spots in the end-to-end, I probably should review the changes in the contracts to get a better sense, but this one LGTM. Thanks!

block_height: height.value(),
block_hash,
next_configuration_number,
cross_messages_hash: et::H256::zero().0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the "beefy"-checkpoint, are we going to also propagate a hash and let the relayer populate the messages inside the checkpoint for submission?

Should we also add a //TODO comment here and assign a ticket to remind ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I was hoping that you guys can tell me in the review where to get this from. Is there a method to call to retrieve all the messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created https://github.com/consensus-shipyard/fendermint/issues/281 to assign a value to this field.

@adlrocha yes, the relayer is expected to retrieve the cross-messages and send them with the checkpoint in https://github.com/consensus-shipyard/fendermint/issues/257

The parent gateway is expected to hash the messages and compare them to the inside the checkpoint.

let power_updates = PowerUpdates(Vec::new());

// TODO: #252: Take the configuration number of the last change.
let next_configuration_number = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the config number for the next configuration to be adopted by the child subnet, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, this will be the configuration number from the latest update stored by the gateway. I started implementing in #272 but ended up being blocked on the lack of a parseable public key in the changeset. So currently no changes are applied, indicated by zero.

.context("failed to get the power table")?;

// TODO #252: Take the next changes from the gateway.
let power_updates = PowerUpdates(Vec::new());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We assume that the updates have already been submitted and queued in the child's gateway through the top-down proof of finality, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this takes the updates placed in the gateway by the top-down syncer.

@aakoshh aakoshh merged commit 68347e4 into main Sep 26, 2023
@aakoshh aakoshh deleted the fm-254-create-checkpoint branch September 26, 2023 12:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExecInterpreter::end construct a BottomUpCheckpoint and add it to the ledger
3 participants