ValidatorSet reporting #4208
ValidatorSet reporting #4208
Conversation
keorn
added
A0-pleasereview 🤓
arkpar
added
A8-looksgood 🦄
|
conflicts |
gavofyork
added
A7-looksgoodcantmerge 🙄
|
Commit squashing on merge is evil... |
keorn
added
A8-looksgood 🦄
|
Makes backporting a hell of a lot easier. Many projects of this size actually expect PRs to be rebased/squashed down prior to merge. |
|
just grumble about conflicts, appreciate the 👎 though 😄 |
| @@ -275,15 +275,17 @@ impl Engine for AuthorityRound { | |||
| // Give one step slack if step is lagging, double vote is still not possible. | |||
| if header_step <= self.step.load(AtomicOrdering::SeqCst) + 1 { | |||
| let proposer_signature = header_signature(header)?; | |||
| let ok_sig = verify_address(&self.step_proposer(header_step), &proposer_signature, &header.bare_hash())?; | |||
| let correct_proposer = self.step_proposer(header_step); | |||
| let ok_sig = verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())?; | |||
There was a problem hiding this comment.
maybe just
if verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())? {
...
}
else {
...
}
| .ok_or("No client!".into()) | ||
| .and_then(|c| c.transact_contract(a, d).map_err(|e| format!("Transaction import error: {}", e))) | ||
| .map(|_| Default::default()); | ||
| *self.provider.write() = Some(provider::Contract::new(self.validators.address, transact)); |
There was a problem hiding this comment.
Is this Default::default() contract really needs to be set as provider when there is no Client? Is this a part of some real logic? Maybe it should be left as None and return error or fail fast with other method?
There was a problem hiding this comment.
Not sure what you mean, its there to conform with the Contract::new type signature (it will produce an empty Vec<u8> here). It means that transact results are discarded.
There was a problem hiding this comment.
Ok, so you upgrade Weak<Client> and modify the state of the &self with some data it returned. If you failed to acquire reference to the Client, self.provider becomes vec![]. How is this empty-vector state differs from this vector being None logically? If it does not, is there really should be 2 states?
There was a problem hiding this comment.
nope, when I failed to get Client then Contract method calling the closure gets Err("No client!"). self.provider is Some(Contract) always after registration, never Vec.
NikVolf
added
A6-mustntgrumble 💦
NikVolf
added
A8-looksgood 🦄
Adds
report_maliciousandreport_benigntoValidatorSet. These can be called by validatorEngines (ones usingValidatorSet) when they see misbehavior.Separates
ValidatorContractfromValidatorSafeContract, where the latter does not have an ability to send transactions.ValidatorContractis able to call reporting contract methodsreportMaliciousandreportBenign, then its up to the contract to do something with that info.Test contract can be found here. Goes after #4189.