Skip to content

Commit 7d96886

Browse files
HoOngEeHoOngEe
committed
Introduce new error type to ActionHandler's verify method
Because custom action has no transaction fee by default, we need to tightly verify custom actions. But previous error type SyntaxError is not enough to express the possible errors in ReportDoubleVote verificaton. Therefore I introduced new error type VerifyError to express the sum type of TrieError, RuntimeError and SyntaxError which can be emerged during the verification.
1 parent edb37a1 commit 7d96886

File tree

8 files changed

+115
-43
lines changed

8 files changed

+115
-43
lines changed

‎core/src/consensus/stake/actions.rs‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ use std::sync::Arc;
1919
use ccrypto::Blake;
2020
use ckey::{recover, Address, Public, Signature};
2121
use client::ConsensusClient;
22+
use cmerkle::TrieError;
2223
use consensus::stake::get_validators;
2324
use consensus::vote_collector::Message;
24-
use ctypes::errors::SyntaxError;
25+
use cstate::VerifyResult;
26+
use ctypes::errors::{RuntimeError, SyntaxError};
2527
use ctypes::CommonParams;
2628
use primitives::{Bytes, H256};
2729
use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp};
@@ -63,11 +65,7 @@ pub enum Action<M: Message> {
6365
}
6466

6567
impl<M: Message> Action<M> {
66-
pub fn verify(
67-
&self,
68-
current_params: &CommonParams,
69-
client: Option<Arc<ConsensusClient>>,
70-
) -> Result<(), SyntaxError> {
68+
pub fn verify(&self, current_params: &CommonParams, client: Option<Arc<ConsensusClient>>) -> VerifyResult<()> {
7169
match self {
7270
Action::TransferCCS {
7371
..
@@ -86,7 +84,8 @@ impl<M: Message> Action<M> {
8684
return Err(SyntaxError::InvalidCustomAction(format!(
8785
"Too long candidate metadata: the size limit is {}",
8886
current_params.max_candidate_metadata_size()
89-
)))
87+
))
88+
.into())
9089
}
9190
}
9291
Action::ChangeParams {
@@ -100,7 +99,8 @@ impl<M: Message> Action<M> {
10099
return Err(SyntaxError::InvalidCustomAction(format!(
101100
"The current network id is {} but the transaction tries to change the network id to {}",
102101
current_network_id, transaction_network_id
103-
)))
102+
))
103+
.into())
104104
}
105105
params.verify().map_err(SyntaxError::InvalidCustomAction)?;
106106
let action = Action::<M>::ChangeParams {
@@ -121,12 +121,13 @@ impl<M: Message> Action<M> {
121121
message2,
122122
} => {
123123
if message1 == message2 {
124-
return Err(SyntaxError::InvalidCustomAction(String::from("messages are duplicated")))
124+
return Err(SyntaxError::InvalidCustomAction(String::from("messages are duplicated")).into())
125125
}
126126
if message1.round() != message2.round() {
127127
return Err(SyntaxError::InvalidCustomAction(String::from(
128128
"The messages are from two different voting rounds",
129-
)))
129+
))
130+
.into())
130131
}
131132

132133
let signer_idx1 = message1.signer_index();
@@ -146,33 +147,33 @@ impl<M: Message> Action<M> {
146147
})?
147148
.parent_hash();
148149

149-
let parent_state = client.state_at(parent_block_hash.into()).ok_or_else(|| {
150-
SyntaxError::InvalidCustomAction(String::from(
151-
"Cannot read state from the given message's parent hash",
152-
))
153-
})?;
150+
let parent_state = client
151+
.state_at(parent_block_hash.into())
152+
.ok_or_else(|| TrieError::InvalidStateRoot(parent_block_hash))?;
154153
let signers_vec: Vec<Public> = get_validators(&parent_state)
155-
.map(|validators| validators.into_iter().map(|val| *val.pubkey()).collect())
156-
.map_err(|_| {
157-
SyntaxError::InvalidCustomAction(String::from("Cannot get validators from parent_state"))
158-
})?;
154+
.map(|validators| validators.into_iter().map(|val| *val.pubkey()).collect())?;
159155

160-
let signer1 = signers_vec.get(signer_idx1).ok_or_else(|| {
161-
SyntaxError::InvalidCustomAction(String::from("Invalid signer index in message1"))
156+
let signer1 = signers_vec.get(signer_idx1).ok_or_else(|| RuntimeError::InvalidValidatorIndex {
157+
idx: signer_idx1,
158+
parent_block_hash,
162159
})?;
163-
let signer2 = signers_vec.get(signer_idx2).ok_or_else(|| {
164-
SyntaxError::InvalidCustomAction(String::from("Invalid signer index in message2"))
160+
let signer2 = signers_vec.get(signer_idx2).ok_or_else(|| RuntimeError::InvalidValidatorIndex {
161+
idx: signer_idx2,
162+
parent_block_hash,
165163
})?;
166164

167165
if signer1 != signer2 {
168166
return Err(SyntaxError::InvalidCustomAction(format!(
169167
"Two messages are signed by different signers: {}, {}",
170168
signer1, signer2
171-
)))
169+
))
170+
.into())
172171
}
173172

174173
if message1.verify(signer1) != Ok(true) || message2.verify(signer2) != Ok(true) {
175-
return Err(SyntaxError::InvalidCustomAction(String::from("Schnorr signature verification fails")))
174+
return Err(
175+
SyntaxError::InvalidCustomAction(String::from("Schnorr signature verification fails")).into()
176+
)
176177
}
177178
}
178179
}

‎core/src/consensus/stake/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::client::ConsensusClient;
2727
use ccrypto::Blake;
2828
use ckey::{public_to_address, recover, Address, Public, Signature};
2929
use consensus::vote_collector::Message;
30-
use cstate::{ActionHandler, StateResult, TopLevelState, TopState, TopStateView};
30+
use cstate::{ActionHandler, StateResult, TopLevelState, TopState, TopStateView, VerifyResult};
3131
use ctypes::errors::{RuntimeError, SyntaxError};
3232
use ctypes::util::unexpected::Mismatch;
3333
use ctypes::{CommonParams, Header};
@@ -136,7 +136,7 @@ impl<M: Message> ActionHandler for Stake<M> {
136136
}
137137
}
138138

139-
fn verify(&self, bytes: &[u8], current_params: &CommonParams) -> Result<(), SyntaxError> {
139+
fn verify(&self, bytes: &[u8], current_params: &CommonParams) -> VerifyResult<()> {
140140
let action = Action::<M>::decode(&UntrustedRlp::new(bytes))
141141
.map_err(|err| SyntaxError::InvalidCustomAction(err.to_string()))?;
142142
let client: Option<Arc<ConsensusClient>> = self.client.read().as_ref().and_then(Weak::upgrade);

‎core/src/error.rs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::fmt;
1919
use cio::IoError;
2020
use ckey::{Address, Error as KeyError};
2121
use cmerkle::TrieError;
22-
use cstate::StateError;
22+
use cstate::{StateError, VerifyError};
2323
use ctypes::errors::{HistoryError, RuntimeError, SyntaxError};
2424
use ctypes::util::unexpected::{Mismatch, OutOfBounds};
2525
use ctypes::BlockNumber;
@@ -299,6 +299,16 @@ impl From<StateError> for Error {
299299
}
300300
}
301301

302+
impl From<VerifyError> for Error {
303+
fn from(err: VerifyError) -> Self {
304+
match err {
305+
VerifyError::Trie(err) => Error::Trie(err),
306+
VerifyError::Runtime(err) => Error::Runtime(err),
307+
VerifyError::Syntax(err) => Error::Syntax(err),
308+
}
309+
}
310+
}
311+
302312
impl From<RuntimeError> for Error {
303313
fn from(err: RuntimeError) -> Self {
304314
Error::Runtime(err)

‎state/src/action_handler/hit.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use primitives::H256;
2121
use rlp::{self, Decodable, Encodable, UntrustedRlp};
2222

2323
use super::{ActionDataKeyBuilder, ActionHandler};
24-
use crate::{StateResult, TopLevelState, TopState, TopStateView};
24+
use crate::{StateResult, TopLevelState, TopState, TopStateView, VerifyResult};
2525

2626
const CUSTOM_ACTION_HANDLER_ID: u64 = 1;
2727

@@ -81,7 +81,7 @@ impl ActionHandler for HitHandler {
8181
Ok(())
8282
}
8383

84-
fn verify(&self, bytes: &[u8], _params: &CommonParams) -> Result<(), SyntaxError> {
84+
fn verify(&self, bytes: &[u8], _params: &CommonParams) -> VerifyResult<()> {
8585
HitAction::decode(&UntrustedRlp::new(bytes))
8686
.map_err(|err| SyntaxError::InvalidCustomAction(err.to_string()))?;
8787
Ok(())

‎state/src/action_handler/mod.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,12 @@ use std::convert::From;
2020

2121
use ccrypto::blake256;
2222
use ckey::{Address, Public};
23-
use ctypes::errors::SyntaxError;
2423
use ctypes::{CommonParams, Header};
2524
use primitives::H256;
2625
use rlp::{Encodable, RlpStream};
2726

2827
use super::TopStateView;
29-
use crate::{StateResult, TopLevelState};
28+
use crate::{StateResult, TopLevelState, VerifyResult};
3029

3130
pub trait ActionHandler: Send + Sync {
3231
fn name(&self) -> &'static str;
@@ -39,7 +38,7 @@ pub trait ActionHandler: Send + Sync {
3938
fee_payer: &Address,
4039
sender_pubkey: &Public,
4140
) -> StateResult<()>;
42-
fn verify(&self, bytes: &[u8], common_params: &CommonParams) -> Result<(), SyntaxError>;
41+
fn verify(&self, bytes: &[u8], common_params: &CommonParams) -> VerifyResult<()>;
4342

4443
fn query(&self, key_fragment: &[u8], state: &TopLevelState) -> StateResult<Option<Vec<u8>>> {
4544
let key = ActionDataKeyBuilder::key_from_fragment(self.handler_id(), key_fragment);

‎state/src/error.rs‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,75 @@
1717
use std::fmt;
1818

1919
use cmerkle::TrieError;
20-
use ctypes::errors::RuntimeError;
20+
use ctypes::errors::{RuntimeError, SyntaxError};
2121

2222
#[derive(Clone, Debug, PartialEq)]
23-
pub enum Error {
23+
pub enum StateError {
2424
Trie(TrieError),
2525
Runtime(RuntimeError),
2626
}
2727

28-
impl fmt::Display for Error {
28+
#[derive(Clone, Debug, PartialEq)]
29+
pub enum VerifyError {
30+
Trie(TrieError),
31+
Runtime(RuntimeError),
32+
Syntax(SyntaxError),
33+
}
34+
35+
impl fmt::Display for StateError {
2936
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
3037
match self {
31-
Error::Trie(err) => err.fmt(f),
32-
Error::Runtime(err) => err.fmt(f),
38+
StateError::Trie(err) => err.fmt(f),
39+
StateError::Runtime(err) => err.fmt(f),
3340
}
3441
}
3542
}
3643

37-
impl From<TrieError> for Error {
44+
impl fmt::Display for VerifyError {
45+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
46+
match self {
47+
VerifyError::Trie(err) => err.fmt(f),
48+
VerifyError::Runtime(err) => err.fmt(f),
49+
VerifyError::Syntax(err) => err.fmt(f),
50+
}
51+
}
52+
}
53+
54+
impl From<TrieError> for StateError {
55+
fn from(err: TrieError) -> Self {
56+
StateError::Trie(err)
57+
}
58+
}
59+
60+
impl From<RuntimeError> for StateError {
61+
fn from(err: RuntimeError) -> Self {
62+
StateError::Runtime(err)
63+
}
64+
}
65+
66+
impl From<TrieError> for VerifyError {
3867
fn from(err: TrieError) -> Self {
39-
Error::Trie(err)
68+
VerifyError::Trie(err)
4069
}
4170
}
4271

43-
impl From<RuntimeError> for Error {
72+
impl From<RuntimeError> for VerifyError {
4473
fn from(err: RuntimeError) -> Self {
45-
Error::Runtime(err)
74+
VerifyError::Runtime(err)
75+
}
76+
}
77+
78+
impl From<SyntaxError> for VerifyError {
79+
fn from(err: SyntaxError) -> Self {
80+
VerifyError::Syntax(err)
81+
}
82+
}
83+
84+
impl From<StateError> for VerifyError {
85+
fn from(err: StateError) -> Self {
86+
match err {
87+
StateError::Trie(err) => VerifyError::Trie(err),
88+
StateError::Runtime(err) => VerifyError::Runtime(err),
89+
}
4690
}
4791
}

‎state/src/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub mod tests;
5151
pub use crate::action_handler::{ActionDataKeyBuilder, ActionHandler, FindActionHandler, HitHandler};
5252
pub use crate::checkpoint::{CheckpointId, StateWithCheckpoint};
5353
pub use crate::db::StateDB;
54-
pub use crate::error::Error as StateError;
54+
pub use crate::error::{StateError, VerifyError};
5555
pub use crate::impls::{ShardLevelState, TopLevelState};
5656
pub use crate::item::account::Account;
5757
pub use crate::item::action_data::ActionData;
@@ -66,3 +66,4 @@ pub use crate::traits::{ShardState, ShardStateView, StateWithCache, TopState, To
6666
use crate::cache::CacheableItem;
6767

6868
pub type StateResult<T> = Result<T, StateError>;
69+
pub type VerifyResult<T> = Result<T, VerifyError>;

‎types/src/errors/runtime_error.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ pub enum Error {
117117
},
118118
SignatureOfInvalidAccount(Address),
119119
InsufficientStakes(Mismatch<u64>),
120+
InvalidValidatorIndex {
121+
idx: usize,
122+
parent_block_hash: H256,
123+
},
120124
}
121125

122126
const ERROR_ID_ASSET_NOT_FOUND: u8 = 1;
@@ -151,6 +155,7 @@ const ERROR_ID_NON_ACTIVE_ACCOUNT: u8 = 30;
151155
const ERROR_ID_FAILED_TO_HANDLE_CUSTOM_ACTION: u8 = 31;
152156
const ERROR_ID_SIGNATURE_OF_INVALID_ACCOUNT: u8 = 32;
153157
const ERROR_ID_INSUFFICIENT_STAKES: u8 = 33;
158+
const ERROR_ID_INVALID_VALIDATOR_INDEX: u8 = 34;
154159

155160
struct RlpHelper;
156161
impl TaggedRlp for RlpHelper {
@@ -190,6 +195,7 @@ impl TaggedRlp for RlpHelper {
190195
ERROR_ID_NON_ACTIVE_ACCOUNT => 3,
191196
ERROR_ID_SIGNATURE_OF_INVALID_ACCOUNT => 2,
192197
ERROR_ID_INSUFFICIENT_STAKES => 3,
198+
ERROR_ID_INVALID_VALIDATOR_INDEX => 3,
193199
_ => return Err(DecoderError::Custom("Invalid RuntimeError")),
194200
})
195201
}
@@ -317,6 +323,10 @@ impl Encodable for Error {
317323
expected,
318324
found,
319325
}) => RlpHelper::new_tagged_list(s, ERROR_ID_INSUFFICIENT_STAKES).append(expected).append(found),
326+
Error::InvalidValidatorIndex {
327+
idx,
328+
parent_block_hash,
329+
} => RlpHelper::new_tagged_list(s, ERROR_ID_INVALID_VALIDATOR_INDEX).append(idx).append(parent_block_hash),
320330
};
321331
}
322332
}
@@ -405,6 +415,10 @@ impl Decodable for Error {
405415
expected: rlp.val_at(1)?,
406416
found: rlp.val_at(2)?,
407417
}),
418+
ERROR_ID_INVALID_VALIDATOR_INDEX => Error::InvalidValidatorIndex {
419+
idx: rlp.val_at(1)?,
420+
parent_block_hash: rlp.val_at(2)?,
421+
},
408422
_ => return Err(DecoderError::Custom("Invalid RuntimeError")),
409423
};
410424
RlpHelper::check_size(rlp, tag)?;
@@ -503,6 +517,9 @@ impl Display for Error {
503517
write!(f, "Signature of invalid account({}) received", address),
504518
Error::InsufficientStakes(mismatch) =>
505519
write!(f, "Insufficient stakes: {}", mismatch),
520+
Error::InvalidValidatorIndex {
521+
idx, parent_block_hash,
522+
} => write!(f, "The validator index {} is invalid at the parent hash {}", idx, parent_block_hash),
506523
}
507524
}
508525
}

0 commit comments

Comments
 (0)