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

runtime: Provide sets of good and bad compute nodes in round results #3726

Merged
merged 4 commits into from
Feb 26, 2021

Conversation

kostko
Copy link
Member

@kostko kostko commented Feb 24, 2021

No description provided.

@kostko kostko added c:runtime/compute Category: runtime compute worker c:roothash Category: root hash service c:breaking/consensus Category: breaking consensus changes c:breaking/runtime Category: breaking runtime changes labels Feb 24, 2021
@kostko kostko force-pushed the kostko/feature/rt-committee-info branch from 5b23d6c to b36439e Compare February 24, 2021 11:37
@kostko kostko requested a review from ptrus February 24, 2021 11:37
@kostko kostko force-pushed the kostko/feature/rt-committee-info branch from b36439e to 589c6a9 Compare February 24, 2021 13:55
@kostko kostko marked this pull request as ready for review February 24, 2021 14:14
@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #3726 (174b48a) into master (8ce562b) will decrease coverage by 0.10%.
The diff coverage is 91.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3726      +/-   ##
==========================================
- Coverage   66.93%   66.83%   -0.11%     
==========================================
  Files         400      400              
  Lines       39822    39829       +7     
==========================================
- Hits        26656    26619      -37     
- Misses       9401     9434      +33     
- Partials     3765     3776      +11     
Impacted Files Coverage Δ
go/consensus/tendermint/apps/roothash/api.go 100.00% <ø> (ø)
go/roothash/api/api.go 83.75% <ø> (ø)
go/runtime/history/prune.go 79.59% <0.00%> (ø)
go/runtime/host/protocol/types.go 42.85% <ø> (ø)
go/worker/compute/executor/committee/node.go 68.20% <66.66%> (ø)
go/runtime/history/db.go 80.62% <71.42%> (+1.38%) ⬆️
go/consensus/tendermint/roothash/roothash.go 71.95% <89.47%> (+4.57%) ⬆️
go/consensus/tendermint/apps/roothash/roothash.go 74.57% <100.00%> (ø)
go/roothash/tests/tester.go 88.79% <100.00%> (+0.07%) ⬆️
go/runtime/history/history.go 72.04% <100.00%> (ø)
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b0e11d...289309d. Read the comment docs.

@kostko kostko force-pushed the kostko/feature/rt-committee-info branch from 589c6a9 to dc4effe Compare February 24, 2021 17:41

// GoodComputeNodes are the public keys of compute nodes that positively contributed to the
// round by replicating the computation correctly.
GoodComputeNodes []signature.PublicKey `json:"good_compute_nodes,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if there's a better alternative naming to use for nodes that submitted correct and invalid results. I thought about it, but don't have a better suggestion though :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about how to name this as well, but didn't come up with anything better. Tendermint uses the term "Byzantine Validators" for the bad ones.

@@ -587,50 +591,41 @@ func (app *rootHashApplication) tryFinalizeExecutorCommits(
return fmt.Errorf("failed to process runtime messages: %w", err)
}

var (
Copy link
Member

Choose a reason for hiding this comment

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

👍

@kostko kostko force-pushed the kostko/feature/rt-committee-info branch from dc4effe to 174b48a Compare February 25, 2021 09:52
@kostko kostko disabled auto-merge February 25, 2021 10:40
@kostko kostko force-pushed the kostko/feature/rt-committee-info branch 3 times, most recently from f993cfd to 98e7097 Compare February 26, 2021 09:25
In addition to the existing runtime message execution results we want to provide
additional information about the last successful round to runtimes. This
introduces a RoundResults structure for this purpose.
Previously only incorrect primary members were slashed and correct backup
members were rewarded. Now, any incorrect members are slashed and any correct
members are rewarded.
This allows the runtime to be aware of how many runtime messages can be emitted
in this round before the round would fail.
@kostko kostko force-pushed the kostko/feature/rt-committee-info branch from 98e7097 to 289309d Compare February 26, 2021 09:44
@kostko kostko merged commit df716b1 into master Feb 26, 2021
@kostko kostko deleted the kostko/feature/rt-committee-info branch February 26, 2021 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/consensus Category: breaking consensus changes c:breaking/runtime Category: breaking runtime changes c:roothash Category: root hash service c:runtime/compute Category: runtime compute worker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants