runtime-sdk: Consensus module #92

ptrus · 2021-04-12T16:36:34Z

This PR:

adds generalized consensus message handling support
- Since consensus emitted messages get executed after the round finishes the results need to be handled in the next round. Modules can now register message hooks that get called at the start of the next round. When emitting a message one now needs to specify a handler to be called after the message is processed (also supports passing arbitrary handler context).
adds low level consensus, and higher level consensus-accounts modules to the SDK
add a general Context trait so that module methods can support both TxContext and DispatchCtx
adds e2e test to test the consensus modules

TODO:

clean the added message handler (just focused on getting it working)
more consensus module stuff (staking txs, querying consensus state)
- staking TXs requiers Add support for obtaining consensus events from runtime oasis-core#3862 probably makes sense deffering into a separate PR
some tests

Only low-level staking support (emitting messages) is available currently. Adding a higher level support for staking (e.g. adding staking to consensus-accounts module) will be done later, as it requires oasisprotocol/oasis-core#3862.

Requires:

codecov · 2021-04-12T16:44:48Z

Codecov Report

Merging #92 (123b667) into main (8baf071) will increase coverage by 3.93%.
The diff coverage is 66.77%.

❗ Current head 123b667 differs from pull request most recent head 48a93bf. Consider uploading reports for the commit 48a93bf to get more accurate results

@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   59.42%   63.36%   +3.93%     
==========================================
  Files          41       38       -3     
  Lines        1649     2001     +352     
==========================================
+ Hits          980     1268     +288     
- Misses        650      733      +83     
+ Partials       19        0      -19

Impacted Files	Coverage Δ
runtime-sdk/src/crypto/signature/secp256k1.rs	`29.03% <0.00%> (ø)`
runtime-sdk/src/dispatcher.rs	`0.00% <0.00%> (ø)`
runtime-sdk/src/module.rs	`17.77% <0.00%> (-4.45%)`	⬇️
runtime-sdk/src/runtime.rs	`0.00% <0.00%> (ø)`
tests/runtimes/simple-consensus/src/lib.rs	`0.00% <0.00%> (ø)`
tests/runtimes/simple-consensus/src/main.rs	`0.00% <0.00%> (ø)`
tests/runtimes/simple-keyvalue/src/keyvalue.rs	`0.00% <ø> (ø)`
tests/runtimes/simple-keyvalue/src/lib.rs	`0.00% <ø> (ø)`
runtime-sdk/src/modules/consensus_accounts/mod.rs	`38.59% <38.59%> (ø)`
runtime-sdk/src/context.rs	`64.59% <51.76%> (-5.90%)`	⬇️
... and 25 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 8baf071...48a93bf. Read the comment docs.

runtime-sdk/src/context.rs

runtime-sdk/src/dispatcher.rs

tests/runtimes/simple-bank/Cargo.toml

tests/runtimes/simple-bank/src/lib.rs

runtime-sdk/src/context.rs

runtime-sdk/src/module.rs

kostko · 2021-04-14T12:53:55Z

runtime-sdk/src/runtime.rs

@@ -19,7 +20,11 @@ pub trait Runtime {
    /// Runtime version.
    const VERSION: version::Version;

-    type Modules: AuthHandler + MigrationHandler + MethodRegistrationHandler + BlockHandler;
+    type Modules: AuthHandler


Is there a reason these are here and not trait bounds on Module. @kostko ?

It could be done either way and we always only use modules here so maybe we can move them (in a subsequent issue/PR of course).

runtime-sdk/src/modules/consensus/mod.rs

runtime-sdk/src/modules/consensus_accounts/mod.rs

kostko · 2021-04-19T09:16:45Z

runtime-sdk/src/dispatcher.rs

+const MODULE: &str = "dispatcher";
+
+/// State schema constants.
+pub mod state {
+    /// Map of message idx to message handlers for messages emitted in previous round.
+    pub const HANDLERS: &[u8] = &[0x01];
+}


We should probably move this under the core module as that one currently has a similar thing for module migration metadata.

runtime-sdk/src/dispatcher.rs

runtime-sdk/src/module.rs

runtime-sdk/src/modules/accounts/mod.rs

runtime-sdk/src/modules/consensus_accounts/mod.rs

runtime-sdk/src/types/address.rs

runtime-sdk/src/modules/consensus_accounts/mod.rs

runtime-sdk/src/context.rs

runtime-sdk/src/modules/consensus/mod.rs

kostko · 2021-04-20T11:41:49Z

runtime-sdk/src/context.rs

@@ -31,21 +27,126 @@ pub enum Mode {
    SimulateTx,
 }

+impl Mode {
+    /// Mode string representation.
+    pub fn to_str(&self) -> &'static str {


You could implement the Display trait for Mode instead and then directly use mode below.

So afaik to use it in the logger context as i do bellow (so that it's present in all log invocations) it needs to be threadsafe ('static, Send, Sync).

So I'm not sure if the display trait implementation will be useful. However I should probably move these strings into constants and use these to also implement the Display trait and maybe rename this method to to_static_str with a comment on why we need it.

Unless i missed something obvious?

pro-wh · 2021-04-27T18:23:41Z

ok, I think I'll have the tx_value stuff in my branch similar to how you've done the tx_auth_info and tx_caller_address, with Options

runtime-sdk/src/context.rs

runtime-sdk/src/modules/consensus/mod.rs

runtime-sdk/src/modules/consensus_accounts/test.rs

runtime-sdk/src/modules/consensus_accounts/types.rs

runtime-sdk/src/types/address.rs

runtime-sdk/src/modules/consensus_accounts/mod.rs

kostko · 2021-04-29T16:07:23Z

runtime-sdk/src/context.rs

+    }
+}
+
+/// Runtime sdk context.


Suggested change

/// Runtime sdk context.

/// Runtime SDK context.

kostko · 2021-04-29T16:14:12Z

tests/e2e/simple_consensus.go

+		Amount:       *quantity.NewFromUint64(50),
+		Denomination: types.Denomination("TEST"),
+	}
+	log.Info("charlie withdrawing")


Should also test with dave (secp256k1).

good idea, done 👍

ptrus force-pushed the ptrus/feature/consensus-sdk branch from 2d76bcc to 02f3163 Compare April 13, 2021 06:39

nhynes reviewed Apr 13, 2021

View reviewed changes

ptrus force-pushed the ptrus/feature/consensus-sdk branch 5 times, most recently from afa2283 to 2be49e8 Compare April 14, 2021 11:55

kostko reviewed Apr 14, 2021

View reviewed changes

ptrus force-pushed the ptrus/feature/consensus-sdk branch 3 times, most recently from aaf3b10 to 9d3a69e Compare April 15, 2021 16:00

kostko reviewed Apr 15, 2021

View reviewed changes

runtime-sdk/src/modules/consensus/mod.rs Outdated Show resolved Hide resolved

runtime-sdk/src/modules/consensus/mod.rs Outdated Show resolved Hide resolved

runtime-sdk/src/modules/consensus/mod.rs Outdated Show resolved Hide resolved

ptrus mentioned this pull request Apr 16, 2021

runtime: Support for block tags oasisprotocol/oasis-core#3848

Merged

ptrus force-pushed the ptrus/feature/consensus-sdk branch 2 times, most recently from 7b1c9ed to 85de3fd Compare April 19, 2021 06:20

kostko reviewed Apr 19, 2021

View reviewed changes

runtime-sdk/src/modules/consensus_accounts/mod.rs Outdated Show resolved Hide resolved

ptrus force-pushed the ptrus/feature/consensus-sdk branch 2 times, most recently from 68d6c6c to 822ad2d Compare April 19, 2021 16:00

kostko reviewed Apr 19, 2021

View reviewed changes

ptrus force-pushed the ptrus/feature/consensus-sdk branch from 822ad2d to ddee57d Compare April 19, 2021 17:28

kostko linked an issue Apr 20, 2021 that may be closed by this pull request

Consensus layer interaction module #11

Closed

kostko reviewed Apr 20, 2021

View reviewed changes

kostko mentioned this pull request Apr 21, 2021

runtime-sdk: Properly handle error chains #100

Merged

ptrus force-pushed the ptrus/feature/consensus-sdk branch 5 times, most recently from cf8e372 to 86b5a75 Compare April 23, 2021 14:30

ptrus force-pushed the ptrus/feature/consensus-sdk branch 6 times, most recently from 91f5525 to 76bb6bb Compare April 27, 2021 13:01

ptrus marked this pull request as ready for review April 27, 2021 13:16

ptrus requested a review from pro-wh as a code owner April 27, 2021 13:16

ptrus force-pushed the ptrus/feature/consensus-sdk branch 3 times, most recently from 4440e5a to a35720c Compare April 27, 2021 15:43

ptrus force-pushed the ptrus/feature/consensus-sdk branch 2 times, most recently from 05e4bf6 to a6f076a Compare April 28, 2021 05:59

kostko reviewed Apr 28, 2021

View reviewed changes

kostko mentioned this pull request Apr 29, 2021

runtime-sdk: gas accounting and estimate query #94

Merged

This was referenced Apr 29, 2021

Runtime-sdk extend testing framework to support advancing rounds #110

Open

Add tests for max messages per runtime context #112

Closed

ptrus force-pushed the ptrus/feature/consensus-sdk branch 2 times, most recently from b6757df to aaa78a9 Compare April 29, 2021 15:49

kostko reviewed Apr 29, 2021

View reviewed changes

kostko approved these changes Apr 29, 2021

View reviewed changes

ptrus added 2 commits April 29, 2021 19:27

runtime-sdk: Consensus module

e0dc6cf

tests: setup consensus module e2e tests

48a93bf

ptrus force-pushed the ptrus/feature/consensus-sdk branch from aaa78a9 to 48a93bf Compare April 29, 2021 17:28

ptrus merged commit 0c7672d into main Apr 29, 2021

ptrus deleted the ptrus/feature/consensus-sdk branch April 29, 2021 18:04

ptrus mentioned this pull request Apr 29, 2021

Add consensus_accounts ts/go clients #113

Closed

pro-wh mentioned this pull request Apr 29, 2021

ts-web/rt: consensus module #114

Closed

kostko mentioned this pull request Aug 25, 2021

Consensus staking support #386

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime-sdk: Consensus module #92

runtime-sdk: Consensus module #92

ptrus commented Apr 12, 2021 •

edited

Loading

codecov bot commented Apr 12, 2021 •

edited

Loading

kostko Apr 14, 2021

kostko Apr 19, 2021

kostko Apr 20, 2021

ptrus Apr 20, 2021 •

edited

Loading

pro-wh commented Apr 27, 2021

kostko Apr 29, 2021

kostko Apr 29, 2021

ptrus Apr 29, 2021

runtime-sdk: Consensus module #92

runtime-sdk: Consensus module #92

Conversation

ptrus commented Apr 12, 2021 • edited Loading

codecov bot commented Apr 12, 2021 • edited Loading

Codecov Report

kostko Apr 14, 2021

Choose a reason for hiding this comment

kostko Apr 19, 2021

Choose a reason for hiding this comment

kostko Apr 20, 2021

Choose a reason for hiding this comment

ptrus Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

pro-wh commented Apr 27, 2021

kostko Apr 29, 2021

Choose a reason for hiding this comment

kostko Apr 29, 2021

Choose a reason for hiding this comment

ptrus Apr 29, 2021

Choose a reason for hiding this comment

ptrus commented Apr 12, 2021 •

edited

Loading

codecov bot commented Apr 12, 2021 •

edited

Loading

ptrus Apr 20, 2021 •

edited

Loading