Skip to content

Export stable methods for public access #807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

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

48 changes: 32 additions & 16 deletions actors/datacap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use num_traits::{FromPrimitive, Zero};
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::runtime::{ActorCode, Runtime};
use fil_actors_runtime::{
actor_error, cbor, ActorContext, ActorError, AsActorError, SYSTEM_ACTOR_ADDR,
actor_error, cbor, restrict_internal_api, ActorContext, ActorError, AsActorError,
SYSTEM_ACTOR_ADDR,
};

pub use self::state::State;
Expand All @@ -42,9 +43,10 @@ lazy_static! {
* BigInt::from(1_000_000_000_000_000_000_000_i128)
);
}
/// Static method numbers for builtin-actor private dispatch.
/// The methods are also expected to be exposed via FRC-XXXX standard calling convention,
/// with numbers determined by name.

/// Datacap actor methods available
/// Some methods are available under 2 method nums -- a static number for "private" builtin actor usage,
/// and via FRC-XXXX calling convention, with number determined by method name.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should drop the private versions of the standard methods, and invoke the exported ones from verifreg. I also think we should do it now (in a follow-up PR).

#808

#[derive(FromPrimitive)]
#[repr(u64)]
pub enum Method {
Expand All @@ -65,6 +67,19 @@ pub enum Method {
Burn = 19,
BurnFrom = 20,
Allowance = 21,
// Method numbers derived from FRC-XXXX standards
NameExported = frc42_dispatch::method_hash!("Name"),
SymbolExported = frc42_dispatch::method_hash!("Symbol"),
TotalSupplyExported = frc42_dispatch::method_hash!("TotalSupply"),
BalanceOfExported = frc42_dispatch::method_hash!("BalanceOf"),
TransferExported = frc42_dispatch::method_hash!("Transfer"),
TransferFromExported = frc42_dispatch::method_hash!("TransferFrom"),
IncreaseAllowanceExported = frc42_dispatch::method_hash!("IncreaseAllowance"),
DecreaseAllowanceExported = frc42_dispatch::method_hash!("DecreaseAllowance"),
RevokeAllowanceExported = frc42_dispatch::method_hash!("RevokeAllowance"),
BurnExported = frc42_dispatch::method_hash!("Burn"),
BurnFromExported = frc42_dispatch::method_hash!("BurnFrom"),
AllowanceExported = frc42_dispatch::method_hash!("Allowance"),
}

pub struct Actor;
Expand Down Expand Up @@ -448,6 +463,7 @@ impl ActorCode for Actor {
where
RT: Runtime,
{
restrict_internal_api(rt, method)?;
// I'm trying to find a fixed template for these blocks so we can macro it.
// Current blockers:
// - the serialize method maps () to CBOR null (we want no bytes instead)
Expand All @@ -465,51 +481,51 @@ impl ActorCode for Actor {
let ret = Self::destroy(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "destroy result")
}
Some(Method::Name) => {
Some(Method::Name) | Some(Method::NameExported) => {
let ret = Self::name(rt)?;
serialize(&ret, "name result")
}
Some(Method::Symbol) => {
Some(Method::Symbol) | Some(Method::SymbolExported) => {
let ret = Self::symbol(rt)?;
serialize(&ret, "symbol result")
}
Some(Method::TotalSupply) => {
Some(Method::TotalSupply) | Some(Method::TotalSupplyExported) => {
let ret = Self::total_supply(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "total_supply result")
}
Some(Method::BalanceOf) => {
Some(Method::BalanceOf) | Some(Method::BalanceOfExported) => {
let ret = Self::balance_of(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "balance_of result")
}
Some(Method::Transfer) => {
Some(Method::Transfer) | Some(Method::TransferExported) => {
let ret = Self::transfer(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "transfer result")
}
Some(Method::TransferFrom) => {
Some(Method::TransferFrom) | Some(Method::TransferFromExported) => {
let ret = Self::transfer_from(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "transfer_from result")
}
Some(Method::IncreaseAllowance) => {
Some(Method::IncreaseAllowance) | Some(Method::IncreaseAllowanceExported) => {
let ret = Self::increase_allowance(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "increase_allowance result")
}
Some(Method::DecreaseAllowance) => {
Some(Method::DecreaseAllowance) | Some(Method::DecreaseAllowanceExported) => {
let ret = Self::decrease_allowance(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "decrease_allowance result")
}
Some(Method::RevokeAllowance) => {
Some(Method::RevokeAllowance) | Some(Method::RevokeAllowanceExported) => {
Self::revoke_allowance(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::default())
}
Some(Method::Burn) => {
Some(Method::Burn) | Some(Method::BurnExported) => {
let ret = Self::burn(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "burn result")
}
Some(Method::BurnFrom) => {
Some(Method::BurnFrom) | Some(Method::BurnFromExported) => {
let ret = Self::burn_from(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "burn_from result")
}
Some(Method::Allowance) => {
Some(Method::Allowance) | Some(Method::AllowanceExported) => {
let ret = Self::allowance(rt, cbor::deserialize_params(params)?)?;
serialize(&ret, "allowance result")
}
Expand Down
39 changes: 37 additions & 2 deletions actors/datacap/tests/datacap_actor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ mod mint {

use fil_actor_datacap::{Actor, Method, MintParams, INFINITE_ALLOWANCE};
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::test_utils::{expect_abort_contains_message, MARKET_ACTOR_CODE_ID};
use fil_actors_runtime::test_utils::{
expect_abort_contains_message, make_identity_cid, MARKET_ACTOR_CODE_ID,
};
use fil_actors_runtime::{STORAGE_MARKET_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR};
use fvm_ipld_encoding::RawBytes;
use std::ops::Sub;
Expand Down Expand Up @@ -62,6 +64,21 @@ mod mint {
h.check_state(&rt);
}

#[test]
fn requires_builtin_caller() {
let (mut rt, h) = make_harness();
let amt = TokenAmount::from_whole(1);
let params = MintParams { to: *ALICE, amount: amt, operators: vec![] };

rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000));
expect_abort_contains_message(
ExitCode::USR_FORBIDDEN,
"must be built-in",
rt.call::<Actor>(Method::Mint as MethodNum, &serialize(&params, "params").unwrap()),
);
h.check_state(&rt);
}

#[test]
fn requires_verifreg_caller() {
let (mut rt, h) = make_harness();
Expand Down Expand Up @@ -188,15 +205,33 @@ mod transfer {
mod destroy {
use crate::{make_harness, ALICE, BOB};
use fil_actor_datacap::DestroyParams;
use fil_actors_runtime::test_utils::{expect_abort_contains_message, ACCOUNT_ACTOR_CODE_ID};
use fil_actors_runtime::test_utils::{
expect_abort_contains_message, make_identity_cid, ACCOUNT_ACTOR_CODE_ID,
};
use fil_actors_runtime::VERIFIED_REGISTRY_ACTOR_ADDR;
use fvm_shared::address::Address;
use fvm_shared::econ::TokenAmount;
use fvm_shared::MethodNum;

use fil_actor_datacap::{Actor, Method};
use fil_actors_runtime::cbor::serialize;
use fvm_shared::error::ExitCode;

#[test]
fn requires_builtin_caller() {
let (mut rt, h) = make_harness();
let amt = TokenAmount::from_whole(1);
let params = DestroyParams { owner: *ALICE, amount: amt };

rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000));
expect_abort_contains_message(
ExitCode::USR_FORBIDDEN,
"must be built-in",
rt.call::<Actor>(Method::Destroy as MethodNum, &serialize(&params, "params").unwrap()),
);
h.check_state(&rt);
}

#[test]
fn only_governor_allowed() {
let (mut rt, h) = make_harness();
Expand Down
1 change: 1 addition & 0 deletions actors/datacap/tests/harness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub struct Harness {

impl Harness {
pub fn construct_and_verify(&self, rt: &mut MockRuntime, registry: &Address) {
rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR);
rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]);
let ret = rt
.call::<DataCapActor>(
Expand Down
1 change: 1 addition & 0 deletions actors/init/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ crate-type = ["cdylib", "lib"]

[dependencies]
fil_actors_runtime = { version = "10.0.0-alpha.1", path = "../../runtime" }
frc42_dispatch = "1.0.0"
fvm_shared = { version = "2.0.0-alpha.2", default-features = false }
fvm_ipld_hamt = "0.5.1"
serde = { version = "1.0.136", features = ["derive"] }
Expand Down
9 changes: 7 additions & 2 deletions actors/init/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
use cid::Cid;
use fil_actors_runtime::runtime::builtins::Type;
use fil_actors_runtime::runtime::{ActorCode, Runtime};
use fil_actors_runtime::{actor_error, cbor, ActorContext, ActorError, SYSTEM_ACTOR_ADDR};
use fil_actors_runtime::{
actor_error, cbor, restrict_internal_api, ActorContext, ActorError, SYSTEM_ACTOR_ADDR,
};
use fvm_ipld_encoding::RawBytes;
use fvm_shared::address::Address;
use fvm_shared::{ActorID, MethodNum, METHOD_CONSTRUCTOR};
Expand All @@ -27,6 +29,8 @@ fil_actors_runtime::wasm_trampoline!(Actor);
pub enum Method {
Constructor = METHOD_CONSTRUCTOR,
Exec = 2,
// Method numbers derived from FRC-XXXX standards
ExecExported = frc42_dispatch::method_hash!("Exec"),
}

/// Init actor
Expand Down Expand Up @@ -102,12 +106,13 @@ impl ActorCode for Actor {
where
RT: Runtime,
{
restrict_internal_api(rt, method)?;
match FromPrimitive::from_u64(method) {
Some(Method::Constructor) => {
Self::constructor(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::default())
}
Some(Method::Exec) => {
Some(Method::Exec) | Some(Method::ExecExported) => {
let res = Self::exec(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::serialize(res)?)
}
Expand Down
63 changes: 62 additions & 1 deletion actors/init/tests/init_actor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use fil_actor_init::testing::check_state_invariants;
use fil_actor_init::{
Actor as InitActor, ConstructorParams, ExecParams, ExecReturn, Method, State,
};
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::runtime::Runtime;
use fil_actors_runtime::test_utils::*;
use fil_actors_runtime::{
Expand All @@ -15,7 +16,7 @@ use fvm_ipld_encoding::RawBytes;
use fvm_shared::address::Address;
use fvm_shared::econ::TokenAmount;
use fvm_shared::error::ExitCode;
use fvm_shared::{HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR};
use fvm_shared::{MethodNum, HAMT_BIT_WIDTH, METHOD_CONSTRUCTOR};
use num_traits::Zero;
use serde::Serialize;

Expand Down Expand Up @@ -287,7 +288,67 @@ fn sending_constructor_failure() {
check_state(&rt);
}

#[test]
fn exec_restricted_correctly() {
let mut rt = construct_runtime();
construct_and_verify(&mut rt);

// set caller to not-builtin
rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000));

// cannot call the unexported method num
let fake_constructor_params =
RawBytes::serialize(ConstructorParams { network_name: String::from("fake_param") })
.unwrap();
let exec_params = ExecParams {
code_cid: *MULTISIG_ACTOR_CODE_ID,
constructor_params: RawBytes::serialize(fake_constructor_params.clone()).unwrap(),
};

expect_abort_contains_message(
ExitCode::USR_FORBIDDEN,
"must be built-in",
rt.call::<InitActor>(
Method::Exec as MethodNum,
&serialize(&exec_params, "params").unwrap(),
),
);

// can call the exported method num

// Assign addresses
let unique_address = Address::new_actor(b"multisig");
rt.new_actor_addr = Some(unique_address);

// Next id
let expected_id = 100;
let expected_id_addr = Address::new_id(expected_id);
rt.expect_create_actor(*MULTISIG_ACTOR_CODE_ID, expected_id);

// Expect a send to the multisig actor constructor
rt.expect_send(
expected_id_addr,
METHOD_CONSTRUCTOR,
RawBytes::serialize(&fake_constructor_params).unwrap(),
TokenAmount::zero(),
RawBytes::default(),
ExitCode::OK,
);

rt.expect_validate_caller_any();

let ret = rt
.call::<InitActor>(Method::ExecExported as u64, &RawBytes::serialize(&exec_params).unwrap())
.unwrap();
let exec_ret: ExecReturn = RawBytes::deserialize(&ret).unwrap();
assert_eq!(unique_address, exec_ret.robust_address, "Robust address does not macth");
assert_eq!(expected_id_addr, exec_ret.id_address, "Id address does not match");
check_state(&rt);
rt.verify();
}

fn construct_and_verify(rt: &mut MockRuntime) {
rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR);
rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]);
let params = ConstructorParams { network_name: "mock".to_string() };
let ret =
Expand Down
1 change: 1 addition & 0 deletions actors/market/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ fil_actors_runtime = { version = "10.0.0-alpha.1", path = "../../runtime"}

anyhow = "1.0.65"
cid = { version = "0.8.3", default-features = false, features = ["serde-codec"] }
frc42_dispatch = "1.0.0"
frc46_token = "1.1.0"
fvm_ipld_bitfield = "0.5.2"
fvm_ipld_blockstore = "0.1.1"
Expand Down
15 changes: 10 additions & 5 deletions actors/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ use fil_actors_runtime::cbor::{deserialize, serialize, serialize_vec};
use fil_actors_runtime::runtime::builtins::Type;
use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime};
use fil_actors_runtime::{
actor_error, cbor, ActorContext, ActorDowncast, ActorError, AsActorError,
BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, CRON_ACTOR_ADDR, DATACAP_TOKEN_ACTOR_ADDR,
REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
actor_error, cbor, restrict_internal_api, ActorContext, ActorDowncast, ActorError,
AsActorError, BURNT_FUNDS_ACTOR_ADDR, CALLER_TYPES_SIGNABLE, CRON_ACTOR_ADDR,
DATACAP_TOKEN_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
VERIFIED_REGISTRY_ACTOR_ADDR,
};

use crate::ext::verifreg::{AllocationID, AllocationRequest};
Expand Down Expand Up @@ -71,6 +72,9 @@ pub enum Method {
OnMinerSectorsTerminate = 7,
ComputeDataCommitment = 8,
CronTick = 9,
// Method numbers derived from FRC-XXXX standards
AddBalanceExported = frc42_dispatch::method_hash!("AddBalance"),
WithdrawBalanceExported = frc42_dispatch::method_hash!("WithdrawBalance"),
}

/// Market Actor
Expand Down Expand Up @@ -1389,16 +1393,17 @@ impl ActorCode for Actor {
where
RT: Runtime,
{
restrict_internal_api(rt, method)?;
match FromPrimitive::from_u64(method) {
Some(Method::Constructor) => {
Self::constructor(rt)?;
Ok(RawBytes::default())
}
Some(Method::AddBalance) => {
Some(Method::AddBalance) | Some(Method::AddBalanceExported) => {
Self::add_balance(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::default())
}
Some(Method::WithdrawBalance) => {
Some(Method::WithdrawBalance) | Some(Method::WithdrawBalanceExported) => {
let res = Self::withdraw_balance(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::serialize(res)?)
}
Expand Down
Loading