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.

Sign up for GitHub

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

Jump to bottom

runtime: Various changes needed for the Runtime SDK #3573

Merged
merged 3 commits into from
Jan 5, 2021
Merged

runtime: Various changes needed for the Runtime SDK #3573

merged 3 commits into from
Jan 5, 2021

Conversation

kostko
Copy link
Member

@kostko kostko commented Dec 16, 2020 •
edited

Also see #3572.

@kostko kostko added the c:breaking/runtime Category: breaking runtime changes label Dec 16, 2020
@kostko kostko force-pushed the kostko/feature/runtime-sdk-updates branch 5 times, most recently from 9b33856 to c27a42b Compare December 18, 2020 12:41
@kostko kostko force-pushed the kostko/feature/runtime-sdk-updates branch from c27a42b to 1147e29 Compare January 4, 2021 12:31
@codecov
Copy link

codecov bot commented Jan 4, 2021 •
edited

Codecov Report

Merging #3573 (0aebbaa) into master (d1da416) will increase coverage by 0.02%.
The diff coverage is 68.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3573      +/-   ##
==========================================
+ Coverage   66.54%   66.56%   +0.02%     
==========================================
  Files         377      377              
  Lines       34574    34606      +32     
==========================================
+ Hits        23006    23037      +31     
+ Misses       8243     8242       -1     
- Partials     3325     3327       +2
Impacted Files Coverage Δ
go/worker/compute/executor/committee/node.go 66.24% <33.33%> (-0.01%) ⬇️
go/runtime/host/protocol/types.go 42.85% <50.00%> (+1.19%) ⬆️
go/runtime/client/api/grpc.go 61.30% <62.50%> (+0.08%) ⬆️
go/runtime/client/client.go 66.25% <73.33%> (+0.47%) ⬆️
go/runtime/client/tests/tester.go 100.00% <100.00%> (ø)
go/common/sgx/aesm/aesm.go 62.61% <0.00%> (-6.55%) ⬇️
go/consensus/tendermint/full/light.go 48.93% <0.00%> (-4.26%) ⬇️
go/worker/common/committee/node.go 70.61% <0.00%> (-3.61%) ⬇️
go/worker/keymanager/worker.go 61.06% <0.00%> (-2.81%) ⬇️
...consensus/tendermint/apps/scheduler/state/state.go 65.90% <0.00%> (-2.28%) ⬇️
... and 28 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 d1da416...0aebbaa. Read the comment docs.

@kostko kostko marked this pull request as ready for review January 4, 2021 16:25
@ptrus ptrus self-requested a review January 4, 2021 17:04
ptrus
ptrus reviewed Jan 5, 2021
View reviewed changes
runtime/src/dispatcher.rs Outdated Show resolved Hide resolved
@kostko
runtime/common/version: Make Version::new a const fn
46b3384 
Also expose members publicly.
@kostko
runtime: Make CheckTx result more generic
2ebc0c9 
The previous definition made it hard for the runtime host to reason about the
returned status code of CheckTx unless it was tied to the result format
defined by the runtime.

This is a breaking change to the Runtime Host Protocol as the
RuntimeCheckTxBatchResponse structure has changed. It also breaks the
transaction::Dispatcher API.
@kostko
go/runtime/client: Add GetEvents to fetch emitted runtime events
0aebbaa
@kostko kostko force-pushed the kostko/feature/runtime-sdk-updates branch from 1147e29 to 0aebbaa Compare January 5, 2021 11:48
ptrus
ptrus approved these changes Jan 5, 2021
View reviewed changes
Copy link
Member

@ptrus ptrus left a comment

Choose a reason for hiding this comment

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

thanks for the changes!

@kostko kostko merged commit 5ca0db6 into master Jan 5, 2021
@kostko kostko deleted the kostko/feature/runtime-sdk-updates branch January 5, 2021 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptrus ptrus ptrus approved these changes

@peterjgilbert peterjgilbert Awaiting requested review from peterjgilbert peterjgilbert is a code owner

@pro-wh pro-wh Awaiting requested review from pro-wh pro-wh is a code owner

@Yawning Yawning Awaiting requested review from Yawning

Assignees
No one assigned
Labels
c:breaking/runtime Category: breaking runtime changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kostko @ptrus