-
Notifications
You must be signed in to change notification settings - Fork 3
Feature: Checkpoint refactor and cross-net message execution #67
Conversation
Co-authored-by: adlrocha <adlrocha@tutamail.com>
Co-authored-by: adlrocha <adlrocha@tutamail.com>
* update cron * fix lint --------- Co-authored-by: willesxm <willeslau@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job, Will. We are close. I think we can simplify the logic a bit (we can go without a lot of the linked HAMTs, I think they add unnecessary complexity). Let me know if my comments are clear enough, if not we can have a quick sync.
Next up:
- A PR including tests targeting this branch.
- A PR with the implementation of Tracking subnet membership from gateway or new membership actor #68
Thanks.
gateway/src/cron.rs
Outdated
#[derive(Clone, Debug, Serialize_tuple, Deserialize_tuple, PartialEq, Eq)] | ||
pub struct CronCheckpoint { | ||
pub epoch: ChainEpoch, | ||
pub validators: ValidatorSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end you can remove validators from here, we will get the validator set from the new data structure introduced here: #68. Thanks!
gateway/src/lib.rs
Outdated
rt.validate_immediate_caller_type(CALLER_TYPES_SIGNABLE.iter())?; | ||
|
||
let msgs = rt.transaction(|st: &mut State, rt| { | ||
// first we check the epoch is the correct one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment is redundant, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it returns the messages to be executed, should not be redundant?
* track validators * add validator check to submit cron * update impl
* track validators * add validator check to submit cron * update impl * weighted vote * Update gateway/src/cron.rs Co-authored-by: adlrocha <adlrocha@tutamail.com> * update method name --------- Co-authored-by: adlrocha <adlrocha@tutamail.com>
* track validators * add validator check to submit cron * update impl * weighted vote * Update gateway/src/cron.rs Co-authored-by: adlrocha <adlrocha@tutamail.com> * update method name * add tests * refactor pending epoches * fix clippy * add more tests --------- Co-authored-by: adlrocha <adlrocha@tutamail.com>
* track validators * add validator check to submit cron * update impl * weighted vote * Update gateway/src/cron.rs Co-authored-by: adlrocha <adlrocha@tutamail.com> * update method name * add tests * refactor pending epoches * fix clippy * add more tests * initial commit * Cross execution (#75) * update bottom up execution * update cross message execution * fix fmt * update review and clean up * check message ordering * Cross execution tests (#76) * fix clippy * fmt code --------- Co-authored-by: adlrocha <adlrocha@tutamail.com>
* track validators * add validator check to submit cron * update impl * weighted vote * Update gateway/src/cron.rs Co-authored-by: adlrocha <adlrocha@tutamail.com> * update method name * add tests * refactor pending epoches * fix clippy * add more tests * initial commit * Cross execution (#75) * update bottom up execution * update cross message execution * fix fmt * update review and clean up * check message ordering * Cross execution tests (#76) * fix clippy * fmt code * generics for cron submission * migrate to sdk * format code * remove wip field * work in progress * local changes * reorg code * update comment * update tests * format code and clippy * fix error --------- Co-authored-by: adlrocha <adlrocha@tutamail.com>
* update queue serialization * remove println * fix fmt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally merging this one 🥳 Future changes and bug fixes will be done in follow-up PRs.
Changes
Submit a new cron checkpoint.
It only accepts submission at multiples of
cron_period
sincegenesis_epoch
, which are set during construction. Each checkpoint will have its number of submissions tracked. The same address cannot submit twice. Once the number of submissions is more than or equal to 2/3 of the total number of validators, the messages will be applied.Each cron checkpoint will check its uniqueness against each other using blake hashing. To do that, we need to make sure the
validators
andtop_down_msgs
inCronCheckpoint
are the same. However, thetop_down_msgs
andvalidators
are vec, they may contain the same content, but their orders are different. In this case, we need to ensure the same order is maintained in the cron checkpoint submission. To ensure we have the same consistent output for different submissions, we require:net_addr
in string ascending order, assumingnet_addr
are unique for the validatorsActor will not perform sorting to save gas. Client should do it, actor just check.
A side note @adlrocha, what execution order do we have to enforce when the messages are applied? Currently it's enforced as the above top down message sorting order.
Update 20/03/2023:
Tests
Not implemented yet, implementing
Issues to be ported to Lotus and go-ipc-types