Backports to beta (#7660)
* Improve handling of RocksDB corruption (#7630) * kvdb-rocksdb: update rust-rocksdb version * kvdb-rocksdb: mark corruptions and attempt repair on db open * kvdb-rocksdb: better corruption detection on open * kvdb-rocksdb: add corruption_file_name const * kvdb-rocksdb: rename mark_corruption to check_for_corruption * Hardening of CSP (#7621) * Fixed delegatecall's from/to (#7568) * Fixed delegatecall's from/to, closes #7166 * added tests for delegatecall traces, #7167 * Light client RPCs (#7603) * Implement registrar. * Implement eth_getCode * Don't wait for providers. * Don't wait for providers. * Fix linting and wasm tests. * Problem: AttachedProtocols don't get registered (#7610) I was investigating issues I am having with Whisper support. I've enabled Whisper on a custom test network and inserted traces into Whisper handler implementation (Network<T> and NetworkProtocolHandler for Network<T>) and I noticed that the handler was never invoked. After further research on this matter, I found out that AttachedProtocol's register function does nothing: https://github.com/paritytech/parity/blob/master/sync/src/api.rs#L172 but there was an implementation originally: 99075ad#diff-5212acb6bcea60e9804ba7b50f6fe6ec and it did the actual expected logic of registering the protocol in the NetworkService. However, as of 16d84f8#diff-5212acb6bcea60e9804ba7b50f6fe6ec ("finished removing ipc") this implementation is gone and only the no-op function is left. Which leads me to a conclusion that in fact Whisper's handler never gets registered in the service and therefore two nodes won't communicate using it. Solution: Resurrect original non-empty `AttachedProtocols.register` implementation Resolves #7566 * Fix Temporarily Invalid blocks handling (#7613) * Handle temporarily invalid blocks in sync. * Fix tests.
This commit is contained in:
committed by
Afri Schoedon
parent
a8fc42d282
commit
fa6a0a6b60
@@ -44,8 +44,7 @@ use bigint::hash::{H256, H520};
|
||||
use semantic_version::SemanticVersion;
|
||||
use parking_lot::{Mutex, RwLock};
|
||||
use unexpected::{Mismatch, OutOfBounds};
|
||||
use util::*;
|
||||
use bytes::Bytes;
|
||||
use util::Address;
|
||||
|
||||
mod finality;
|
||||
|
||||
@@ -291,9 +290,11 @@ struct EpochVerifier {
|
||||
|
||||
impl super::EpochVerifier<EthereumMachine> for EpochVerifier {
|
||||
fn verify_light(&self, header: &Header) -> Result<(), Error> {
|
||||
// Validate the timestamp
|
||||
verify_timestamp(&*self.step, header_step(header)?)?;
|
||||
// always check the seal since it's fast.
|
||||
// nothing heavier to do.
|
||||
verify_external(header, &self.subchain_validators, &*self.step, |_| {})
|
||||
verify_external(header, &self.subchain_validators)
|
||||
}
|
||||
|
||||
fn check_finality_proof(&self, proof: &[u8]) -> Option<Vec<H256>> {
|
||||
@@ -317,7 +318,7 @@ impl super::EpochVerifier<EthereumMachine> for EpochVerifier {
|
||||
//
|
||||
// `verify_external` checks that signature is correct and author == signer.
|
||||
if header.seal().len() != 2 { return None }
|
||||
otry!(verify_external(header, &self.subchain_validators, &*self.step, |_| {}).ok());
|
||||
otry!(verify_external(header, &self.subchain_validators).ok());
|
||||
|
||||
let newly_finalized = otry!(finality_checker.push_hash(header.hash(), header.author().clone()).ok());
|
||||
finalized.extend(newly_finalized);
|
||||
@@ -327,16 +328,6 @@ impl super::EpochVerifier<EthereumMachine> for EpochVerifier {
|
||||
}
|
||||
}
|
||||
|
||||
// Report misbehavior
|
||||
#[derive(Debug)]
|
||||
#[allow(dead_code)]
|
||||
enum Report {
|
||||
// Malicious behavior
|
||||
Malicious(Address, BlockNumber, Bytes),
|
||||
// benign misbehavior
|
||||
Benign(Address, BlockNumber),
|
||||
}
|
||||
|
||||
fn header_step(header: &Header) -> Result<usize, ::rlp::DecoderError> {
|
||||
UntrustedRlp::new(&header.seal().get(0).expect("was either checked with verify_block_basic or is genesis; has 2 fields; qed (Make sure the spec file has a correct genesis seal)")).as_val()
|
||||
}
|
||||
@@ -355,34 +346,35 @@ fn is_step_proposer(validators: &ValidatorSet, bh: &H256, step: usize, address:
|
||||
step_proposer(validators, bh, step) == *address
|
||||
}
|
||||
|
||||
fn verify_external<F: Fn(Report)>(header: &Header, validators: &ValidatorSet, step: &Step, report: F)
|
||||
-> Result<(), Error>
|
||||
{
|
||||
let header_step = header_step(header)?;
|
||||
|
||||
fn verify_timestamp(step: &Step, header_step: usize) -> Result<(), BlockError> {
|
||||
match step.check_future(header_step) {
|
||||
Err(None) => {
|
||||
trace!(target: "engine", "verify_block_external: block from the future");
|
||||
report(Report::Benign(*header.author(), header.number()));
|
||||
return Err(BlockError::InvalidSeal.into())
|
||||
trace!(target: "engine", "verify_timestamp: block from the future");
|
||||
Err(BlockError::InvalidSeal.into())
|
||||
},
|
||||
Err(Some(oob)) => {
|
||||
trace!(target: "engine", "verify_block_external: block too early");
|
||||
return Err(BlockError::TemporarilyInvalid(oob).into())
|
||||
// NOTE This error might be returned only in early stage of verification (Stage 1).
|
||||
// Returning it further won't recover the sync process.
|
||||
trace!(target: "engine", "verify_timestamp: block too early");
|
||||
Err(BlockError::TemporarilyInvalid(oob).into())
|
||||
},
|
||||
Ok(_) => {
|
||||
let proposer_signature = header_signature(header)?;
|
||||
let correct_proposer = validators.get(header.parent_hash(), header_step);
|
||||
let is_invalid_proposer = *header.author() != correct_proposer ||
|
||||
!verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())?;
|
||||
Ok(_) => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
if is_invalid_proposer {
|
||||
trace!(target: "engine", "verify_block_external: bad proposer for step: {}", header_step);
|
||||
Err(EngineError::NotProposer(Mismatch { expected: correct_proposer, found: header.author().clone() }))?
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
fn verify_external(header: &Header, validators: &ValidatorSet) -> Result<(), Error> {
|
||||
let header_step = header_step(header)?;
|
||||
|
||||
let proposer_signature = header_signature(header)?;
|
||||
let correct_proposer = validators.get(header.parent_hash(), header_step);
|
||||
let is_invalid_proposer = *header.author() != correct_proposer ||
|
||||
!verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())?;
|
||||
|
||||
if is_invalid_proposer {
|
||||
trace!(target: "engine", "verify_block_external: bad proposer for step: {}", header_step);
|
||||
Err(EngineError::NotProposer(Mismatch { expected: correct_proposer, found: header.author().clone() }))?
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -655,26 +647,38 @@ impl Engine<EthereumMachine> for AuthorityRound {
|
||||
/// Check the number of seal fields.
|
||||
fn verify_block_basic(&self, header: &Header) -> Result<(), Error> {
|
||||
if header.number() >= self.validate_score_transition && *header.difficulty() >= U256::from(U128::max_value()) {
|
||||
Err(From::from(BlockError::DifficultyOutOfBounds(
|
||||
return Err(From::from(BlockError::DifficultyOutOfBounds(
|
||||
OutOfBounds { min: None, max: Some(U256::from(U128::max_value())), found: *header.difficulty() }
|
||||
)))
|
||||
} else {
|
||||
Ok(())
|
||||
)));
|
||||
}
|
||||
|
||||
// TODO [ToDr] Should this go from epoch manager?
|
||||
// If yes then probably benign reporting needs to be moved further in the verification.
|
||||
let set_number = header.number();
|
||||
|
||||
match verify_timestamp(&*self.step, header_step(header)?) {
|
||||
Err(BlockError::InvalidSeal) => {
|
||||
self.validators.report_benign(header.author(), set_number, header.number());
|
||||
Err(BlockError::InvalidSeal.into())
|
||||
}
|
||||
Err(e) => Err(e.into()),
|
||||
Ok(()) => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Do the step and gas limit validation.
|
||||
fn verify_block_family(&self, header: &Header, parent: &Header) -> Result<(), Error> {
|
||||
let step = header_step(header)?;
|
||||
|
||||
let parent_step = header_step(parent)?;
|
||||
// TODO [ToDr] Should this go from epoch manager?
|
||||
let set_number = header.number();
|
||||
|
||||
// Ensure header is from the step after parent.
|
||||
if step == parent_step
|
||||
|| (header.number() >= self.validate_step_transition && step <= parent_step) {
|
||||
trace!(target: "engine", "Multiple blocks proposed for step {}.", parent_step);
|
||||
|
||||
self.validators.report_malicious(header.author(), header.number(), header.number(), Default::default());
|
||||
self.validators.report_malicious(header.author(), set_number, header.number(), Default::default());
|
||||
Err(EngineError::DoubleVote(header.author().clone()))?;
|
||||
}
|
||||
|
||||
@@ -687,7 +691,7 @@ impl Engine<EthereumMachine> for AuthorityRound {
|
||||
let skipped_primary = step_proposer(&*self.validators, &parent.hash(), s);
|
||||
// Do not report this signer.
|
||||
if skipped_primary != me {
|
||||
self.validators.report_benign(&skipped_primary, header.number(), header.number());
|
||||
self.validators.report_benign(&skipped_primary, set_number, header.number());
|
||||
}
|
||||
// Stop reporting once validators start repeating.
|
||||
if !reported.insert(skipped_primary) { break; }
|
||||
@@ -702,9 +706,8 @@ impl Engine<EthereumMachine> for AuthorityRound {
|
||||
// fetch correct validator set for current epoch, taking into account
|
||||
// finality of previous transitions.
|
||||
let active_set;
|
||||
|
||||
let (validators, set_number) = if self.immediate_transitions {
|
||||
(&*self.validators, header.number())
|
||||
let validators = if self.immediate_transitions {
|
||||
&*self.validators
|
||||
} else {
|
||||
// get correct validator set for epoch.
|
||||
let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) {
|
||||
@@ -722,21 +725,12 @@ impl Engine<EthereumMachine> for AuthorityRound {
|
||||
}
|
||||
|
||||
active_set = epoch_manager.validators().clone();
|
||||
(&active_set as &_, epoch_manager.epoch_transition_number)
|
||||
};
|
||||
|
||||
// always report with "self.validators" so that the report actually gets
|
||||
// to the contract.
|
||||
let report = |report| match report {
|
||||
Report::Benign(address, block_number) =>
|
||||
self.validators.report_benign(&address, set_number, block_number),
|
||||
Report::Malicious(address, block_number, proof) =>
|
||||
self.validators.report_malicious(&address, set_number, block_number, proof),
|
||||
&active_set as &_
|
||||
};
|
||||
|
||||
// verify signature against fixed list, but reports should go to the
|
||||
// contract itself.
|
||||
verify_external(header, validators, &*self.step, report)
|
||||
verify_external(header, validators)
|
||||
}
|
||||
|
||||
fn genesis_epoch_data(&self, header: &Header, call: &Call) -> Result<Vec<u8>, String> {
|
||||
@@ -1059,8 +1053,7 @@ mod tests {
|
||||
assert!(engine.verify_block_family(&header, &parent_header).is_ok());
|
||||
assert!(engine.verify_block_external(&header).is_ok());
|
||||
header.set_seal(vec![encode(&5usize).into_vec(), encode(&(&*signature as &[u8])).into_vec()]);
|
||||
assert!(engine.verify_block_family(&header, &parent_header).is_ok());
|
||||
assert!(engine.verify_block_external(&header).is_err());
|
||||
assert!(engine.verify_block_basic(&header).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1200,3 +1193,4 @@ mod tests {
|
||||
AuthorityRound::new(params, machine).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -192,7 +192,7 @@ mod tests {
|
||||
header.set_number(2);
|
||||
header.set_parent_hash(client.chain_info().best_block_hash);
|
||||
// `reportBenign` when the designated proposer releases block from the future (bad clock).
|
||||
assert!(client.engine().verify_block_external(&header).is_err());
|
||||
assert!(client.engine().verify_block_basic(&header).is_err());
|
||||
// Seal a block.
|
||||
client.engine().step();
|
||||
assert_eq!(client.chain_info().best_block_number, 1);
|
||||
|
||||
@@ -1406,7 +1406,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn should_not_trace_delegatecall() {
|
||||
fn should_trace_delegatecall_properly() {
|
||||
init_log();
|
||||
|
||||
let mut state = get_temp_state();
|
||||
@@ -1426,7 +1426,7 @@ mod tests {
|
||||
}.sign(&secret(), None);
|
||||
|
||||
state.init_code(&0xa.into(), FromHex::from_hex("6000600060006000600b618000f4").unwrap()).unwrap();
|
||||
state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()).unwrap();
|
||||
state.init_code(&0xb.into(), FromHex::from_hex("60056000526001601ff3").unwrap()).unwrap();
|
||||
let result = state.apply(&info, &machine, &t, true).unwrap();
|
||||
|
||||
let expected_trace = vec![FlatTrace {
|
||||
@@ -1441,23 +1441,23 @@ mod tests {
|
||||
call_type: CallType::Call,
|
||||
}),
|
||||
result: trace::Res::Call(trace::CallResult {
|
||||
gas_used: U256::from(721), // in post-eip150
|
||||
gas_used: U256::from(736), // in post-eip150
|
||||
output: vec![]
|
||||
}),
|
||||
}, FlatTrace {
|
||||
trace_address: vec![0].into_iter().collect(),
|
||||
subtraces: 0,
|
||||
action: trace::Action::Call(trace::Call {
|
||||
from: "9cce34f7ab185c7aba1b7c8140d620b4bda941d6".into(),
|
||||
to: 0xa.into(),
|
||||
from: 0xa.into(),
|
||||
to: 0xb.into(),
|
||||
value: 0.into(),
|
||||
gas: 32768.into(),
|
||||
input: vec![],
|
||||
call_type: CallType::DelegateCall,
|
||||
}),
|
||||
result: trace::Res::Call(trace::CallResult {
|
||||
gas_used: 3.into(),
|
||||
output: vec![],
|
||||
gas_used: 18.into(),
|
||||
output: vec![5],
|
||||
}),
|
||||
}];
|
||||
|
||||
|
||||
@@ -74,13 +74,23 @@ pub struct Call {
|
||||
|
||||
impl From<ActionParams> for Call {
|
||||
fn from(p: ActionParams) -> Self {
|
||||
Call {
|
||||
from: p.sender,
|
||||
to: p.address,
|
||||
value: p.value.value(),
|
||||
gas: p.gas,
|
||||
input: p.data.unwrap_or_else(Vec::new),
|
||||
call_type: p.call_type,
|
||||
match p.call_type {
|
||||
CallType::DelegateCall => Call {
|
||||
from: p.address,
|
||||
to: p.code_address,
|
||||
value: p.value.value(),
|
||||
gas: p.gas,
|
||||
input: p.data.unwrap_or_else(Vec::new),
|
||||
call_type: p.call_type,
|
||||
},
|
||||
_ => Call {
|
||||
from: p.sender,
|
||||
to: p.address,
|
||||
value: p.value.value(),
|
||||
gas: p.gas,
|
||||
input: p.data.unwrap_or_else(Vec::new),
|
||||
call_type: p.call_type,
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user