Call emitInitiateChange #76
Conversation
DemiMarie
force-pushed
the
call_emitInitiateChange
branch
from
fdf2314 to
500abf7
Compare
There was a problem hiding this comment.
I'm confused: Isn't on_epoch_begin only called after an InitiateChange event has been found? (See this comment for my attempt to track this through the code.) The doc comment also reads like finalizeChange is meant to be called after InitiateChange.
| @@ -49,7 +49,7 @@ use unexpected::{Mismatch, OutOfBounds}; | |||
|
|
|||
| mod finality; | |||
| mod randomness; | |||
| mod util; | |||
There was a problem hiding this comment.
Should we make that pub(crate)?
| .map_err(::engines::EngineError::FailedSystemCall) | ||
| .map_err(Into::into) | ||
| .map_err(::error::Error::from)?; |
There was a problem hiding this comment.
Is that second map_err necessary? Isn't that ?'s job? (Also below.)
There was a problem hiding this comment.
I'm confused: Isn't
on_epoch_beginonly called after anInitiateChangeevent has been found? (See this comment for my attempt to track this through the code.) The doc comment also reads likefinalizeChangeis meant to be called afterInitiateChange.
You are correct. This really needs to go in ValidatorSet::on_new_block, which doesn’t currently exist. Should I create it, or drop the code in AuthorityRound::on_new_block?
There was a problem hiding this comment.
I'd put it in AuthorityRound::on_new_block for now; it doesn't need anything from SafeContract, does it? Not sure whether that would conceptually be the right place for it.
There was a problem hiding this comment.
@afck It needs ValidatorSafeContract::contract_address.
There was a problem hiding this comment.
You're right; probably ValidatorSet::on_new_block would make sense.
There was a problem hiding this comment.
@DemiMarie Unfortunately no, because this getter doesn't know which validator is currently selected.
There was a problem hiding this comment.
With #75 we should be able to make sure that calls always apply to the block we're currently authoring, i.e. only if we're selected.
| .as_ref() | ||
| .and_then(Weak::upgrade) | ||
| .ok_or_else(|| ::error::Error::from("No client!"))?; | ||
| let bound_contract = super::super::authority_round::util::BoundContract::bind(&*client, BlockId::Latest, self.contract_address); |
It is now called by the new method `ValidatorSet::on_new_block()`.
Closes #71.