Skip to content

Commit

Permalink
Update account actor and params defaults/checks (#587)
Browse files Browse the repository at this point in the history
* Update account actor and update empty params

* add commit tag comment

* Update tests for new params default

* Update error message

* bump commit

* Remove unused import from merge
  • Loading branch information
austinabell authored Jul 30, 2020
1 parent 25203cb commit f80cfab
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 47 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion blockchain/chain_sync/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ mod tests {
let (bls, secp) = construct_messages();

let expected_root =
Cid::from_raw_cid("bafy2bzacecujyfvb74s7xxnlajidxpgcpk6abyatk62dlhgq6gcob3iixhgom")
Cid::from_raw_cid("bafy2bzacebx7t56l6urh4os4kzar5asc5hmbhl7so6sfkzcgpjforkwylmqxa")
.unwrap();

let root = compute_msg_meta(cs.chain_store.blockstore(), &[bls], &[secp]).unwrap();
Expand Down
1 change: 1 addition & 0 deletions vm/actor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ fil_types = { path = "../../types" }
derive_builder = "0.9"
byteorder = "1.3.4"
ahash = "0.4"
base64 = "0.12.1"

[dev-dependencies]
db = { path = "../../node/db" }
Expand Down
14 changes: 7 additions & 7 deletions vm/actor/src/builtin/account/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use ipld_blockstore::BlockStore;
use num_derive::FromPrimitive;
use num_traits::FromPrimitive;
use runtime::{ActorCode, Runtime};
use vm::{ActorError, ExitCode, MethodNum, Serialized, METHOD_CONSTRUCTOR};
use vm::{actor_error, ActorError, ExitCode, MethodNum, Serialized, METHOD_CONSTRUCTOR};

// * Updated to specs-actors commit: 4784ddb8e54d53c118e63763e4efbcf0a419da28

/// Account actor methods available
#[derive(FromPrimitive)]
Expand All @@ -33,10 +35,8 @@ impl Actor {
match address.protocol() {
Protocol::Secp256k1 | Protocol::BLS => {}
protocol => {
return Err(rt.abort(
ExitCode::ErrIllegalArgument,
format!("address must use BLS or SECP protocol, got {}", protocol),
));
return Err(actor_error!(ErrIllegalArgument;
"address must use BLS or SECP protocol, got {}", protocol));
}
}
rt.create(&State { address })?;
Expand Down Expand Up @@ -74,9 +74,9 @@ impl ActorCode for Actor {
Some(Method::PubkeyAddress) => {
check_empty_params(params)?;
let addr = Self::pubkey_address(rt)?;
Ok(Serialized::serialize(addr).unwrap())
Ok(Serialized::serialize(addr)?)
}
_ => Err(rt.abort(ExitCode::SysErrInvalidMethod, "Invalid method")),
None => Err(actor_error!(SysErrInvalidMethod; "Invalid method")),
}
}
}
16 changes: 8 additions & 8 deletions vm/actor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,27 @@ mod util;

pub use self::builtin::*;
pub use self::util::*;
pub use vm::{ActorState, DealID, Serialized};
pub use vm::{actor_error, ActorError, ActorState, DealID, ExitCode, Serialized};

use cid::Cid;
use encoding::Error as EncodingError;
use ipld_blockstore::BlockStore;
use ipld_hamt::{BytesKey, Error as HamtError, Hamt};
use num_bigint::BigUint;
use unsigned_varint::decode::Error as UVarintError;

const HAMT_BIT_WIDTH: u8 = 5;

type EmptyType = [u8; 0];
const EMPTY_VALUE: EmptyType = [];

/// Deal weight
type DealWeight = BigUint;

/// Used when invocation requires parameters to be an empty array of bytes
#[inline]
fn check_empty_params(params: &Serialized) -> Result<(), EncodingError> {
params.deserialize::<[u8; 0]>().map(|_| ())
fn check_empty_params(params: &Serialized) -> Result<(), ActorError> {
if !params.is_empty() {
Err(actor_error!(ErrSerialization;
"params expected to be empty, was: {}", base64::encode(params.bytes())))
} else {
Ok(())
}
}

/// Create a hamt configured with constant bit width.
Expand Down
8 changes: 4 additions & 4 deletions vm/actor/src/util/set.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2020 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

use crate::{BytesKey, EmptyType, EMPTY_VALUE, HAMT_BIT_WIDTH};
use crate::{BytesKey, HAMT_BIT_WIDTH};
use cid::Cid;
use ipld_blockstore::BlockStore;
use ipld_hamt::{Error, Hamt};
Expand Down Expand Up @@ -41,13 +41,13 @@ where
#[inline]
pub fn put(&mut self, key: BytesKey) -> Result<(), String> {
// Set hamt node to array root
Ok(self.0.set(key, EMPTY_VALUE)?)
Ok(self.0.set(key, ())?)
}

/// Checks if key exists in the set.
#[inline]
pub fn has(&self, key: &[u8]) -> Result<bool, String> {
Ok(self.0.get::<_, EmptyType>(key)?.is_some())
Ok(self.0.get::<_, ()>(key)?.is_some())
}

/// Deletes key from set.
Expand All @@ -68,7 +68,7 @@ where
// iterator should be Box<dyn Error> to not convert to String and lose exit code
Ok(self
.0
.for_each(|s, _: EmptyType| f(s).map_err(|e| e.to_string()))?)
.for_each(|s, _: ()| f(s).map_err(|e| e.to_string()))?)
}

/// Collects all keys from the set into a vector.
Expand Down
16 changes: 8 additions & 8 deletions vm/interpreter/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ where
if msg_gas_cost > msg.gas_limit() {
return Ok(ApplyRet {
msg_receipt: MessageReceipt {
return_data: Serialized::empty(),
return_data: Serialized::default(),
exit_code: ExitCode::SysErrOutOfGas,
gas_used: 0,
},
Expand All @@ -224,7 +224,7 @@ where
Err(_) => {
return Ok(ApplyRet {
msg_receipt: MessageReceipt {
return_data: Serialized::empty(),
return_data: Serialized::default(),
exit_code: ExitCode::SysErrSenderInvalid,
gas_used: 0,
},
Expand All @@ -237,7 +237,7 @@ where
if from_act.code != *ACCOUNT_ACTOR_CODE_ID {
return Ok(ApplyRet {
msg_receipt: MessageReceipt {
return_data: Serialized::empty(),
return_data: Serialized::default(),
exit_code: ExitCode::SysErrSenderInvalid,
gas_used: 0,
},
Expand All @@ -250,7 +250,7 @@ where
if msg.sequence() != from_act.sequence {
return Ok(ApplyRet {
msg_receipt: MessageReceipt {
return_data: Serialized::empty(),
return_data: Serialized::default(),
exit_code: ExitCode::SysErrSenderStateInvalid,
gas_used: 0,
},
Expand All @@ -265,7 +265,7 @@ where
if from_act.balance < total_cost {
return Ok(ApplyRet {
msg_receipt: MessageReceipt {
return_data: Serialized::empty(),
return_data: Serialized::default(),
exit_code: ExitCode::SysErrSenderStateInvalid,
gas_used: 0,
},
Expand Down Expand Up @@ -318,7 +318,7 @@ where
if let Err(e) = rt.charge_gas(rt.price_list().on_chain_return_value(ret_data.len()))
{
act_err = Some(e);
ret_data = Serialized::empty();
ret_data = Serialized::default();
}
}
if rt.gas_used() < 0 {
Expand Down Expand Up @@ -390,9 +390,9 @@ where
match res {
Ok(mut rt) => match vm_send(&mut rt, msg, gas_cost) {
Ok(ser) => (ser, Some(rt), None),
Err(actor_err) => (Serialized::empty(), Some(rt), Some(actor_err)),
Err(actor_err) => (Serialized::default(), Some(rt), Some(actor_err)),
},
Err(e) => (Serialized::empty(), None, Some(e)),
Err(e) => (Serialized::default(), None, Some(e)),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions vm/message/tests/message_json_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ fn message_json_annotations() {
"GasPrice": "9",
"GasLimit": 8,
"Method": 7,
"Params": "gA=="
"Params": ""
},
"signed": {
"Message": {
Expand All @@ -97,7 +97,7 @@ fn message_json_annotations() {
"GasPrice": "9",
"GasLimit": 8,
"Method": 7,
"Params": "gA=="
"Params": ""
},
"Signature": {
"Type": 2,
Expand Down
18 changes: 1 addition & 17 deletions vm/src/method.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2020 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

use super::EMPTY_ARR_BYTES;
use encoding::{de, from_slice, ser, serde_bytes, to_vec, Cbor, Error as EncodingError};
use serde::{Deserialize, Serialize};
use std::ops::Deref;
Expand All @@ -15,23 +14,13 @@ pub const METHOD_SEND: MethodNum = 0;
pub const METHOD_CONSTRUCTOR: MethodNum = 1;

/// Serialized bytes to be used as parameters into actor methods
#[derive(Clone, PartialEq, Debug, Serialize, Deserialize, Hash, Eq)]
#[derive(Clone, PartialEq, Debug, Serialize, Deserialize, Hash, Eq, Default)]
#[serde(transparent)]
pub struct Serialized {
#[serde(with = "serde_bytes")]
bytes: Vec<u8>,
}

impl Default for Serialized {
/// Default serialized bytes is an empty array serialized
#[inline]
fn default() -> Self {
Self {
bytes: EMPTY_ARR_BYTES.clone(),
}
}
}

impl Cbor for Serialized {}

impl Deref for Serialized {
Expand All @@ -47,11 +36,6 @@ impl Serialized {
Self { bytes }
}

/// Empty bytes constructor. Used for empty return values.
pub fn empty() -> Self {
Self { bytes: Vec::new() }
}

/// Contructor for encoding Cbor encodable structure
pub fn serialize<O: ser::Serialize>(obj: O) -> Result<Self, EncodingError> {
Ok(Self {
Expand Down

0 comments on commit f80cfab

Please sign in to comment.