Make Round dyn-safe

[fjarri]

• Bump serde-encoded-bytes to 0.2
• Rename Payload::try_to_typed() and Artifact::try_to_typed() to downcast() (for uniformity with Box and BoxedRound)
• Fix a copy-paste artifact in a comment
• Round methods take dyn CryptoRngCore. This lead to an avalanche of changes, see below.

dyn-safe Round

I started with adding ?Sized to CryptoRngCore in Round methods and removing BoxedRng. For now it is a separate commit. But with that, ObjectSafeRound became extremely close to Round, so I tried removing it completely and make Round itself dyn-safe.

As a result:

• serializer/deserializer args are merged into BoxedFormat (since we need both in Misbehave, even if the round itself only needs a serializer to serialize messages)
• Round::finalize has self: Box<Self> as the first parameter instead of just self

[coveralls]

Pull Request Test Coverage Report for Build 13443457877

Details

• 140 of 185 (75.68%) changed or added relevant lines in 9 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.5%) to 72.893%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
manul/src/combinators/misbehave.rs	23	25	92.0%
manul/src/protocol/round.rs	10	12	83.33%
manul/src/session/echo.rs	7	9	77.78%
manul/src/session/session.rs	15	18	83.33%
manul/src/session/evidence.rs	8	18	44.44%
manul/src/combinators/chain.rs	33	45	73.33%
manul/src/protocol/boxed_round.rs	24	38	63.16%

Files with Coverage Reduction	New Missed Lines	%
manul/src/session/evidence.rs	1	53.88%

Totals
Change from base Build 13401865846:	-0.5%
Covered Lines:	2162
Relevant Lines:	2966

---

💛 - Coveralls

[dvdplm]

Remove Payload::empty() and instead have receive_message() return Option? The reason for the asymmetry between payloads and artifacts is that receive_message() usually returns a payload, so in most cases users would have to write Some(); while for the artifacts it's the opposite, so it's convenient to be able to just return None.

The only comment I have on this is that we have a fair bit of work to do to design our error handling – in particular what to do about panics and our usage of asserts. Do you think we should return Result here? Is there a risk that we'll end up with Return<Option<T>, OurError> down the line?

Rename object_safe.rs module and ObjectSafeRound, which is obsolete terminology, to something aligning with the new one, which is "dyn compatible"?

I think they now refer to it as "dyn safety". I don't think it's a great name but I think we should use it.

Since crypto-bigint now supports dyn RngCore args in all methods, should we get rid of BoxedRng, and just take dyn RngCore too everywhere?

Sounds reasonable.

[fjarri]

The only comment I have on this is that we have a fair bit of work to do to design our error handling – in particular what to do about panics and our usage of asserts.

We only have a few outside of dev. Specifically in RoundId methods we can perhaps return LocalError.

Do you think we should return Result here? Is there a risk that we'll end up with Return<Option, OurError> down the line?

What method are you referring to?

[dvdplm]

• Round methods take dyn CryptoRngCore.

I haven't read the code yet, but can you elaborate on why you prefer dyn CryptoRngCore to impl CryptoRngCore? Was there an issue linked to this I should read perhaps?

[fjarri]

Dyn-safe traits cannot have impl arguments. The Round in this PR basically takes place of ObjectSafeRound which we use in Box<dyn>.

[dvdplm]

LGTM

[dvdplm]

Maybe do something like this to help with the wonky formatting here?

Suggested change

	Ok(FinalizeOutcome::AnotherRound(BoxedRound::new_dynamic(ChainedRound::<
	Id,
	T,
	> {
	state: ChainState::Protocol2(round),
	})))
	let round = entry_point2.make_round(rng, &shared_randomness, &id)?;
	let round = BoxedRound::new_dynamic(ChainedRound::<_, T> {
	state: ChainState::Protocol2(round),
	});
	Ok(FinalizeOutcome::AnotherRound(round))