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.

Sign up for GitHub

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

Jump to bottom

Create state machine consensus & add tests #4162

Merged
merged 22 commits into from
Jun 30, 2023
Merged

Create state machine consensus & add tests #4162

merged 22 commits into from
Jun 30, 2023

Conversation

AurelienFT
Copy link
Contributor

@AurelienFT AurelienFT commented Jun 27, 2023 •
edited
Loading

  • document all added functions
  • try in sandbox /simulation/labnet
  • unit tests on the added/changed features
    • make tests compile
    • make tests pass
  • add logs allowing easy debugging in case the changes caused problems
  • if the API has changed, update the API specification

Fix #4018 and #4107

@AurelienFT AurelienFT requested a review from damip June 27, 2023 15:15
@AurelienFT
Copy link
Contributor Author

@damip That is my first attempt. There is still things that I don't like especially now that I update instead of delete/readd in block_statuses I have some clone to do because elements are still in the hashmap and I fought with the borrow checker. I can check to remove these clone after we validate the way I took to avoid loosing time. Tried in labnet works well.

@AurelienFT AurelienFT changed the title Create state machine consensus Create state machine consensus & add tests Jun 28, 2023
@AurelienFT
Copy link
Contributor Author

Tests are passing on my side will try to improve them but doesn't block review @damip

AurelienFT
AurelienFT commented Jun 29, 2023
View reviewed changes
massa-consensus-worker/src/state/blocks_state.rs Outdated Show resolved Hide resolved
massa-consensus-worker/src/state/blocks_state.rs Outdated Show resolved Hide resolved
massa-consensus-worker/src/state/blocks_state.rs Outdated Show resolved Hide resolved
massa-consensus-worker/src/state/blocks_state.rs Outdated Show resolved Hide resolved
@AurelienFT AurelienFT requested a review from damip June 29, 2023 15:26
damip
damip approved these changes Jun 30, 2023
View reviewed changes
Copy link
Member

@damip damip left a comment

Choose a reason for hiding this comment

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

Still some async tests to port but overall LGTM. Good job !!

@AurelienFT AurelienFT merged commit 86bc4c5 into testnet_24 Jun 30, 2023
This was referenced Jul 3, 2023
Closed
Closed
@AurelienFT AurelienFT deleted the create_state_machine_consensus branch July 4, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damip damip damip 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
@AurelienFT @damip