Stateful entry points #68
Conversation
fjarri
force-pushed
the
stateful-entry-points
branch
from
041ce21 to
ab9893e
Compare
Pull Request Test Coverage Report for Build 11902736853Details
💛 - Coveralls |
fjarri
force-pushed
the
stateful-entry-points
branch
3 times, most recently
from
e331cc7 to
6139888
Compare
fjarri
force-pushed
the
stateful-entry-points
branch
from
da76ef6 to
c805dbb
Compare
fjarri
force-pushed
the
stateful-entry-points
branch
2 times, most recently
from
0727d12 to
2a00e23
Compare
manul/Cargo.toml
Outdated
| rand = { version = "0.8", default-features = false } | ||
| serde_asn1_der = "0.8" | ||
| criterion = "0.5" | ||
| serde-persistent-deserializer = "0.3" | ||
| postcard = { version = "1", default-features = false, features = ["alloc"] } | ||
| serde_json = { version = "1", default-features = false, features = ["alloc"] } | ||
| tracing-subscriber = { version = "0.3", features = ["env-filter"] } |
There was a problem hiding this comment.
I have tinkered a bit with a crate called test-log. It's just a simple little wrapper around tracing and provides an alternative #[test] macro that sets up tracing behind the scenes such that a tracing-enabled test is just #[test] fn some_test(){ assert!(some_call()) } and any calls to info!/debug!/trace! show up nicely colored when passing RUSTLOG=<level> cargo t -- --nocapture.
Maybe worth using?
There was a problem hiding this comment.
Looks interesting, we can avoid adding a tracing-enabled run_sync() in #71
manul/src/protocol/round.rs
Outdated
| /// The default behavior: sends broadcasts and receives echoed messages, or does neither. | ||
| /// | ||
| /// That is, this node will be a part of the echo round if [`Round::make_echo_broadcast`] generates a message. | ||
| Default, |
There was a problem hiding this comment.
You didn't like Full for this? :)
There was a problem hiding this comment.
I am not sold on it, but maybe it's not that important since it'll probably never get specified explicitly anyway, being the default. Let's discuss naming in a separate PR.
manul/src/session/session.rs
Outdated
| } else { | ||
| let echo_round_participation = round.as_ref().echo_round_participation(); | ||
|
|
||
| let round_sends_echo_broadcast = !echo_broadcast.payload().is_none(); |
There was a problem hiding this comment.
My brain stumbles a bit on double negations ("not is none"). Would using is_some() work here?
There was a problem hiding this comment.
It's a method of EchoBroadcast, not Option, so there's currently no is_some(). Perhaps there should be. (also I think this is a change from #68)
manul/src/tests/partial_echo.rs
Outdated
| for (_id, report) in reports { | ||
| assert!(matches!(report.outcome, SessionOutcome::Result(_))); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's nice to see how clean this test is. :)
fjarri
force-pushed
the
stateful-entry-points
branch
from
2a00e23 to
53ceede
Compare
Stacked on top of #67
This PR makes
EntryPointtypes stateful - that is, this removes the need forEntryPoint::Inputsbecause theEntryPoint-implementing type becomes the inputs itself. Fixes #64.Benefits:
Roundtypes, only theEntryPointimpl can be publicsynedrioninconstructors.rs) with additional consistency checks or convenience methodsAlso added
Round::may_produce_result(). Fixes #66