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

Skip replication if there is a (cross)validation error #718

Conversation

bchamagne
Copy link
Member

@bchamagne bchamagne commented Nov 25, 2022

Description

In the distributed workflow, if there is an error in the validation stamp or in the cross validation stamp, then do not send the replication messages to the storage nodes, and directly notify the error to the welcome node.

notify_error used to be an internal event but we had to change this to a function to be able to use it from the replication.state_enter callback.

Fixes #696

IMPORTANT NOTE: The scenario :insufficient_funds is not checked in the distributed_workflow. This means a transaction from an address with insufficient funds will not mark the stamps as invalid and the transaction will be replicated to the storage nodes where they'll be discarded.

  • check for insufficient funds

Type of change

  • Bug fix

How Has This Been Tested?

Tests added to the suite.
Scenario tested on the transaction builder (with 3 nodes): Creation of a token with an invalid JSON. You should see the usual error and you should see a "skipped replication" in the logs.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@samuelmanzanera samuelmanzanera added bug Something isn't working mining Involve transaction validation and mining core team Assigned to the core team labels Nov 28, 2022
and remove the cross validation inconsistency in the test on validation stamp
Copy link
Member

@Neylix Neylix left a comment

Choose a reason for hiding this comment

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

Working well 👍
Just a remark to be consistent with the coding style of the project

lib/archethic/mining/validation_context.ex Outdated Show resolved Hide resolved
validation_stamp will have an error if it's the case
@samuelmanzanera samuelmanzanera merged commit 962cfee into archethic-foundation:develop Dec 2, 2022
tenmoves pushed a commit to tenmoves/archethic-node that referenced this pull request Dec 6, 2022
…dation#718)

It also includes:
* stop the FSM in case of consensus_not_reached
* Validation will check for sufficient funds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core team Assigned to the core team mining Involve transaction validation and mining
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not send replication message if validation has error
3 participants