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

Add timestamp in node validation election #1590

Conversation

wassimans
Copy link

Description

This pull request addresses an issue with the validation node election process. Currently, the election is based on transaction data and a daily nonce seed, which causes repeated selection of the same validation nodes for identical transactions throughout the day. If one of the validation nodes, especially the coordinator, has an incorrect state or is unable to retrieve required data, the transaction will fail validation for the entire day.

The solution implemented here introduces a timestamp into the election process to ensure that the elected nodes vary with each transaction attempt. The timestamp used is from the StartMining message, sent by the welcome node, which serves as a reference timestamp to diversify node election.

Fixes #1564

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

The following tests were conducted to validate this change:

  • Ensuring the seed sorting value changes with changing transactions, and/or ref_timestamp
  • Ensuring the seed sorting value doesn't change for identical parameters (transaction, ref_timestamp)
  • Ensuring that validation nodes election changes with changing transactions and/or ref_timestamp
  • Ensuring that validation nodes election doesn't change for identical transactions and ref_timestamp

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

@wassimans wassimans added feature New feature request mining Involve transaction validation and mining labels Nov 5, 2024
@wassimans wassimans force-pushed the feature/add-timestamp-in-node-election branch from 3acc2df to 8b6d24d Compare November 5, 2024 15:42
@wassimans wassimans requested review from bchamagne, Neylix and samuelmanzanera and removed request for Neylix and samuelmanzanera November 5, 2024 15:57
Copy link
Contributor

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

LGTM

lib/archethic/mining.ex Outdated Show resolved Hide resolved
@wassimans wassimans force-pushed the feature/add-timestamp-in-node-election branch 2 times, most recently from 382fb45 to 22aedab Compare November 7, 2024 09:37
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.

I'm also wondering, in the ValidationContext, when a node cross validate the stamp, it ensure the timestamp is valid. This function could evolve since the timestamp should be the ref_timestamp shared by all validations node

lib/archethic.ex Show resolved Hide resolved
lib/archethic/election.ex Outdated Show resolved Hide resolved
lib/archethic/mining.ex Outdated Show resolved Hide resolved
@wassimans wassimans force-pushed the feature/add-timestamp-in-node-election branch 4 times, most recently from 4c0b0e8 to 76eb7ee Compare November 13, 2024 08:34
@wassimans
Copy link
Author

I'm also wondering, in the ValidationContext, when a node cross validate the stamp, it ensure the timestamp is valid. This function could evolve since the timestamp should be the ref_timestamp shared by all validations node

I now check for equality between the node's validation time and the corss validation time.

For consistency, I also modified the following test/archethic/mining/validation_context_test.exs:252, from "should validate even if validation_time and cross_validation_time are in different oracle bucket" to "should not validate if validation_time and cross_validation_time are in different oracle bucket" and made sure ":timestamp" is present in the inconsistencies table.

The test in test/archethic/mining/validation_context_test.exs:252 ("should validate with a valid validation stamp"), should handle the happy path for us without change.

@wassimans wassimans force-pushed the feature/add-timestamp-in-node-election branch from 3912073 to 7aeb6e0 Compare November 15, 2024 17:01
@wassimans wassimans requested a review from Neylix November 15, 2024 17:18
@Neylix Neylix force-pushed the feature/add-timestamp-in-node-election branch from 4557466 to b212caf Compare November 21, 2024 14:32
@Neylix Neylix merged commit e187ef7 into archethic-foundation:develop Nov 21, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request mining Involve transaction validation and mining
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add timestamp in the validation node election
3 participants