-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix Proof of work existence for node transaction #681
Conversation
@@ -1026,7 +1025,7 @@ defmodule Archethic.Mining.ValidationContext do | |||
|
|||
_ -> | |||
Transaction.verify_origin_signature?(tx, pow) and | |||
SharedSecrets.has_origin_public_key?(pow) |
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.
,,
@@ -1026,7 +1025,7 @@ defmodule Archethic.Mining.ValidationContext do | |||
|
|||
_ -> | |||
Transaction.verify_origin_signature?(tx, pow) and | |||
SharedSecrets.has_origin_public_key?(pow) | |||
pow in ProofOfWork.list_origin_public_keys_candidates(tx) |
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.
unit tests
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.
There is already tests for this check:
test "should get inconsistency when the proof of work is not in authorized keys" do |
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.
Description
This PR aims to resolve a bug introduced in 0aa0577.
Because node transaction doesn't leverage existing origin key for their first transaction. We have to use existing list origin key candidates to avoid code repetition.
Type of change
Checklist: