Create a small proposal for a execution consensus #1703
Conversation
krhubert
force-pushed
the
feature/execution-consensus
branch
2 times, most recently
from
6510dba
to
e3269c4
Compare
krhubert
force-pushed
the
feature/execution-consensus
branch
from
e3269c4
to
1266989
Compare
There was a problem hiding this comment.
But if we do that it means that we might have an issue with the following workflow:
- the network has 3 runners registered
- node A creates an execution with
confirmations = 2*n/3withnthe number of runners >confirmations = 2 - node B unregister
- node C creates the execution with the same parameter but here,
nis different so >confirmations = 1.333 - 2 different executions will be created and none of them will ever be processed
With the solution proposed in #1705, the same workflow will look like
- the network has 3 runners registered
- node A creates an execution (signers < 2n/3 ==> 1 < 2) > still pending execution
- node B unregister
- node C creates the execution (same hash), (signers > 2n/3 ==> 2 > 1.333) > execution created
Here the execution is the same and can pass.
(this minimal confirmation could be another feature that can be added and optional later where users can "discard" execution if they don't reach a minimal number of confirmations but that's a different feature.)
| @@ -75,6 +75,9 @@ message CreateExecutionRequest { | |||
| string price = 10 [ | |||
| (gogoproto.moretags) = 'validate:"coins"' | |||
| ]; | |||
|
|
|||
| // confs is the number of conrimations the exeuction needs to have before chaingin status to passed | |||
| int64 confs = 11; | |||
There was a problem hiding this comment.
let's call that "confirmations" and fix the typo in the comment
| @@ -119,4 +122,9 @@ message Execution { | |||
| int64 blockHeight = 15 [ | |||
| (gogoproto.moretags) = 'hash:"-"' | |||
| ]; | |||
|
|
|||
| // confs is the number of conrimations the exeuction needs to have before chaingin status to passed | |||
| int64 confs = 16 [ | |||
There was a problem hiding this comment.
same here, to rename and fix typo
|
|
||
| // confs is the number of conrimations the exeuction needs to have before chaingin status to passed | ||
| int64 confs = 16 [ | ||
| (gogoproto.moretags) = 'hash:"-"' |
There was a problem hiding this comment.
If we add this data in the execution, then I think the confirmations should be part of the hash, 2 executions should be different if they have 2 different consensuses.
| if !ctx.IsCheckTx() { | ||
| M.InProgress.Add(1) | ||
| var count int64 | ||
| if exec.Confs > 0 && execExist { |
There was a problem hiding this comment.
don't understand why we only add the signer if the execution already exists. What happens to the first signer who actually paid more than the other because he wrote more data. This one is not added to the list and will not be rewarded for leading this execution.
|
The point of skipping the confs on hash calculation is to disable modification of this value aftre exec creation. So the two workflow won't happen, because you can't edit confs. The execution is created by the first signer na then the confs is upadted. The exec creator is not treated as a voter because we need a confirmation from others in network. I need to know, who and when pays for exeuction (in case of process/runner/user) who and when gets reward after exeuction completed/failed (in case of process/runner/user) So the execution can be created by process (orchestrator), inside runner, or just with cli (as I see it) |
|
closing in favor of #1705 |
An updated version of execution consensus.
To be honest I'm not sure how to test it with our e2e tests.
I find a need to write a directly inside a keeper, so we reduce the testing surface in e2e tests.