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

Cron submit tests #73

Merged
merged 13 commits into from
Mar 23, 2023
Merged

Cron submit tests #73

merged 13 commits into from
Mar 23, 2023

Conversation

cryptoAtwill
Copy link
Collaborator

Changes

  • Adding unit tests to set_membership and submit_cron.
  • Found an edge case where the cron submission execution might be stuck, see execute_next_cron_epoch for more details.

Tests

cargo test --all

@cryptoAtwill cryptoAtwill requested a review from adlrocha March 21, 2023 04:49
@cryptoAtwill cryptoAtwill changed the base branch from main to cron_impl March 21, 2023 10:56
@cryptoAtwill cryptoAtwill changed the base branch from cron_impl to weighted_vote March 21, 2023 11:05
adlrocha
adlrocha suggested changes Mar 21, 2023
View reviewed changes
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.

The test are great for an initial coverage. My only concern is the external execution method, I don't think we need it as mentioned in the comments. Let me know if it is clear or you want to discuss sync/

gateway/src/lib.rs Outdated Show resolved Hide resolved
Base automatically changed from weighted_vote to cron_impl March 21, 2023 14:04
adlrocha
adlrocha reviewed Mar 22, 2023
View reviewed changes
gateway/src/lib.rs Outdated Show resolved Hide resolved
adlrocha
adlrocha reviewed Mar 22, 2023
View reviewed changes
gateway/src/lib.rs Outdated Show resolved Hide resolved
adlrocha
adlrocha approved these changes Mar 22, 2023
View reviewed changes
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.

Amzing job, I think we can merge. Quick question, once this is merged, would you rather have a final pass to the cron_submit branch and merge it to main, or do you prefer to get the bottom-up refactor in in this batch?

@cryptoAtwill
Copy link
Collaborator Author

@adlrocha I think working on cron_impl first. If merged to main, likely downstream needs to be updated. There could be changes in bottom up, we can update golang and ipc-client one shot.

@cryptoAtwill cryptoAtwill merged commit d1d09ae into cron_impl Mar 23, 2023
@cryptoAtwill cryptoAtwill deleted the cron_submit_tests branch March 23, 2023 03:08
adlrocha added a commit that referenced this pull request Apr 10, 2023
@cryptoAtwill @willeslau @adlrocha
Feature: Checkpoint refactor and cross-net message execution (#67)
bcb5eb0 
* add cron fields

* fmt code

* Update gateway/src/state.rs

Co-authored-by: adlrocha <adlrocha@tutamail.com>

* Update gateway/src/types.rs

Co-authored-by: adlrocha <adlrocha@tutamail.com>

* update cron (#65)

* update cron

* fix lint

---------

Co-authored-by: willesxm <willeslau@gmail.com>

* add submit cron impl

* add more checks

* add some todo

* derive total validators

* add todo

* specify rust tool chain

* add tests

* support abort

* simplify impl

* Track validators (#70)

* track validators

* add validator check to submit cron

* update impl

* Weighted vote (#71)

* 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>

* Cron submit tests (#73)

* 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>

* Refactor checkpoints (#74)

* 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>

* cargo fmt

* Vote checkpoints (#81)

* 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>

* rename checkpoints to bottomup and topdown

* fix tests

* use CrossMsg for topdown checkpoint

* fix checkpoint (#85)

* update queue serialization (#86)

* update queue serialization

* remove println

* fix fmt

* genesis_epoch bottomup checkpoints to zero

* fix test with new genesis checkpoint

---------

Co-authored-by: willesxm <willeslau@gmail.com>
Co-authored-by: adlrocha <adlrocha@tutamail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adlrocha adlrocha adlrocha approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cryptoAtwill @adlrocha