-
Notifications
You must be signed in to change notification settings - Fork 1k
[N3] Proof of node #4304
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
base: master-n3
Are you sure you want to change the base?
[N3] Proof of node #4304
Conversation
|
Please @neo-project/core take a look to this PR, we should decide the right values for PoW and frequency. Maybe we should move this values to policy: Difficult and frequency |
| if (!state.Registered) | ||
| throw new Exception("Only registered candidates are availables"); | ||
|
|
||
| if (proofOfWork.ToString().StartsWith("0x0000")) // TODO: Decide the proper value |
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.
Why do we need a proof of work at all here? What does it prove?
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.
I believe it is for all committee , not only CN
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.
Cn could be set every time he is the primary
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.
Why do we need a proof of work at all here? What does it prove?
Proof that the committee has a node
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.
Well, this doesn't prove there is a node. I can separate this code from the node. My point is that a transaction is sufficient. Adding any calculations would be a serious regression for Neo and it wouldn't add any benefit.
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.
I also think we don't need to introduce proof of work.
What's your proposal? Regular tx is easy to fake. I thought an idea but it's incompatible with huge amount of committee (outside 21).
If committee produce a signature of the previous block, the primary can choose this rpc signatures and use them as a proof when they persist the block.
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.
@shargon @erikzhang , do you know? Double speakers would be perfect for that because we could, every round, make a committee to be a speaker and do round robin on that.
So, every member would need to be ready. Double speakers do that without any problems because they would be fallback. Based on node's statistics we would know the truth.
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.
That's only work for 21, no for outside 21
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.
I believe that if a node deliberately pretends to be online while refusing to participate in the consensus process, there is essentially no reliable way for us to detect such behavior. Therefore, the primary purpose of “proof of node” should not be to identify malicious non-participation. Instead, it should focus on preventing accidental misconfigurations by candidates—such as forgetting to run the node, failing to start the consensus module, or unintentionally launching multiple consensus instances.
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.
Then only a tx is good for you, and maybe a counter of how many view changes?
|
Could you give a detailed description for this feature ? |
|
vncoelho
left a comment
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.
@shargon , how about information for current state of MPT?
The problem I see is that almost all the information can be easily accessed via RPC. That's the reason for Proof of Work. |
| } | ||
|
|
||
| [ContractMethod(CpuFee = 1 << 15, RequiredCallFlags = CallFlags.States)] | ||
| private void SetProofOfNodeDifficulty(ApplicationEngine engine, ulong value) |
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.
@roman-khimov if difficulty is set to 0xFFFFFFFFFFFFFFFF you don't need a 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.
I do not know, @shargon,
I do not support this PoW here.
I can support it on transactions to improve ordering.
I can support this of P2P propagation priorities.
But a PoW here, as @roman-khimov said, can be done by a different software and do not represent the readiness of a committee.
I do not understand your reasoning very well and the motivation behind this.
I would prefer, as I emphasized in the proposal, some up-to-date signatures from commit payloads, each of them specific to the node.
This at least says, in the same way as PoW, that committee is running some software and signing txs.
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.
A small proof of work to proof that you have the right hardware and it was not done manually. Is not for every blocks, so I don't see any problem
|
Conflicts |
|
Fix conflicts on shargon#18 |
Fix conflicts
Description
Close #4143
This PR will require a transaction from the DBFT plugin to ensure the node life.
Type of change
How Has This Been Tested?
Checklist: