Proof of concept exported API for Account actor (filecoin-project#797)

anorth · shamb0 · commit fbb3613aade8 · 2023-01-31T11:26:13.000+05:30
diff --git a/actors/account/src/lib.rs b/actors/account/src/lib.rs
@@ -13,7 +13,7 @@ use num_traits::FromPrimitive;
 use crate::types::AuthenticateMessageParams;
 use fil_actors_runtime::builtin::singletons::SYSTEM_ACTOR_ADDR;
 use fil_actors_runtime::runtime::{ActorCode, Runtime};
-use fil_actors_runtime::{actor_error, ActorError};
+use fil_actors_runtime::{actor_error, restrict_internal_api, ActorError};
 use fil_actors_runtime::{cbor, ActorDowncast};
 
 pub use self::state::State;
@@ -32,6 +32,7 @@ pub enum Method {
     Constructor = METHOD_CONSTRUCTOR,
     PubkeyAddress = 2,
     AuthenticateMessage = 3,
+    AuthenticateMessageExported = frc42_dispatch::method_hash!("AuthenticateMessage"),
     UniversalReceiverHook = frc42_dispatch::method_hash!("Receive"),
 }
 
@@ -108,6 +109,8 @@ 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)?)?;
@@ -117,7 +120,7 @@ impl ActorCode for Actor {
                 let addr = Self::pubkey_address(rt)?;
                 Ok(RawBytes::serialize(addr)?)
             }
-            Some(Method::AuthenticateMessage) => {
+            Some(Method::AuthenticateMessage) | Some(Method::AuthenticateMessageExported) => {
                 Self::authenticate_message(rt, cbor::deserialize_params(params)?)?;
                 Ok(RawBytes::default())
             }
diff --git a/actors/account/tests/account_actor_test.rs b/actors/account/tests/account_actor_test.rs
@@ -19,13 +19,9 @@ use fil_actors_runtime::test_utils::*;
 #[test]
 fn construction() {
     fn construct(addr: Address, exit_code: ExitCode) {
-        let mut rt = MockRuntime {
-            receiver: Address::new_id(100),
-            caller: *SYSTEM_ACTOR_ADDR,
-            caller_type: *SYSTEM_ACTOR_CODE_ID,
-            ..Default::default()
-        };
-        rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]);
+        let mut rt = MockRuntime { receiver: Address::new_id(100), ..Default::default() };
+        rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR);
+        rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]);
 
         if exit_code.is_success() {
             rt.call::<AccountActor>(
@@ -62,13 +58,9 @@ fn construction() {
 
 #[test]
 fn token_receiver() {
-    let mut rt = MockRuntime {
-        receiver: Address::new_id(100),
-        caller: *SYSTEM_ACTOR_ADDR,
-        caller_type: *SYSTEM_ACTOR_CODE_ID,
-        ..Default::default()
-    };
-    rt.expect_validate_caller_addr(vec![*SYSTEM_ACTOR_ADDR]);
+    let mut rt = MockRuntime { receiver: Address::new_id(100), ..Default::default() };
+    rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR);
+    rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]);
 
     let param = Address::new_secp256k1(&[2; fvm_shared::address::SECP_PUB_LEN]).unwrap();
     rt.call::<AccountActor>(
@@ -77,6 +69,7 @@ fn token_receiver() {
     )
     .unwrap();
 
+    rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000));
     rt.expect_validate_caller_any();
     let ret = rt.call::<AccountActor>(
         Method::FungibleTokenReceiverHook as MethodNum,
@@ -86,86 +79,15 @@ fn token_receiver() {
     assert_eq!(RawBytes::default(), ret.unwrap());
 }
 
-fn check_state(rt: &MockRuntime) {
-    let test_address = Address::new_id(1000);
-    let (_, acc) = check_state_invariants(&rt.get_state(), &test_address);
-    acc.assert_empty();
-}
-
-macro_rules! account_constructor_tests {
-    ($($name:ident: $value:expr,)*) => {
-        $(
-            #[test]
-            fn $name() {
-                let (addr, exit_code) = $value;
-
-                let mut rt = MockRuntime {
-                    receiver: fvm_shared::address::Address::new_id(100),
-                    caller: SYSTEM_ACTOR_ADDR.clone(),
-                    caller_type: SYSTEM_ACTOR_CODE_ID.clone(),
-                    ..Default::default()
-                };
-                rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]);
-
-                if exit_code.is_success() {
-                    rt.call::<AccountActor>(1, &RawBytes::serialize(addr).unwrap()).unwrap();
-
-                    let state: State = rt.get_state();
-                    assert_eq!(state.address, addr);
-                    rt.expect_validate_caller_any();
-
-                    let pk: Address = rt
-                        .call::<AccountActor>(2, &RawBytes::default())
-                        .unwrap()
-                        .deserialize()
-                        .unwrap();
-                    assert_eq!(pk, addr);
-
-                    check_state(&rt);
-                } else {
-                    expect_abort(
-                        exit_code,
-                        rt.call::<AccountActor>(1,&RawBytes::serialize(addr).unwrap())
-                    )
-                }
-                rt.verify();
-            }
-        )*
-    }
-}
-
-account_constructor_tests! {
-    happy_construct_secp256k1_address: (
-        Address::new_secp256k1(&[2; fvm_shared::address::SECP_PUB_LEN]).unwrap(),
-        ExitCode::OK
-    ),
-    happy_construct_bls_address: (
-        Address::new_bls(&[1; fvm_shared::address::BLS_PUB_LEN]).unwrap(),
-        ExitCode::OK
-    ),
-    fail_construct_id_address: (
-        Address::new_id(1),
-        ExitCode::USR_ILLEGAL_ARGUMENT
-    ),
-    fail_construct_actor_address: (
-        Address::new_actor(&[1, 2, 3]),
-        ExitCode::USR_ILLEGAL_ARGUMENT
-    ),
-}
-
 #[test]
 fn authenticate_message() {
-    let mut rt = MockRuntime {
-        receiver: Address::new_id(100),
-        caller: SYSTEM_ACTOR_ADDR,
-        caller_type: *SYSTEM_ACTOR_CODE_ID,
-        ..Default::default()
-    };
+    let mut rt = MockRuntime { receiver: Address::new_id(100), ..Default::default() };
+    rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR);
 
     let addr = Address::new_secp256k1(&[2; fvm_shared::address::SECP_PUB_LEN]).unwrap();
     rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]);
-
-    rt.call::<AccountActor>(1, &RawBytes::serialize(addr).unwrap()).unwrap();
+    rt.call::<AccountActor>(Method::Constructor as MethodNum, &RawBytes::serialize(addr).unwrap())
+        .unwrap();
 
     let state: State = rt.get_state();
     assert_eq!(state.address, addr);
@@ -174,26 +96,56 @@ fn authenticate_message() {
         RawBytes::serialize(AuthenticateMessageParams { signature: vec![], message: vec![] })
             .unwrap();
 
+    // Valid signature
     rt.expect_validate_caller_any();
     rt.expect_verify_signature(ExpectedVerifySig {
         sig: Signature::new_secp256k1(vec![]),
         signer: addr,
         plaintext: vec![],
         result: Ok(()),
     });
-    assert_eq!(RawBytes::default(), rt.call::<AccountActor>(3, &params).unwrap());
+    assert_eq!(
+        RawBytes::default(),
+        rt.call::<AccountActor>(Method::AuthenticateMessage as MethodNum, &params).unwrap()
+    );
+    rt.verify();
 
+    // Invalid signature
     rt.expect_validate_caller_any();
     rt.expect_verify_signature(ExpectedVerifySig {
         sig: Signature::new_secp256k1(vec![]),
         signer: addr,
         plaintext: vec![],
         result: Err(anyhow!("bad signature")),
     });
-    assert_eq!(
+    expect_abort_contains_message(
         ExitCode::USR_ILLEGAL_ARGUMENT,
-        rt.call::<AccountActor>(3, &params).unwrap_err().exit_code()
+        "bad signature",
+        rt.call::<AccountActor>(Method::AuthenticateMessage as MethodNum, &params),
     );
-
     rt.verify();
+
+    // Invalid caller of internal method number
+    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::<AccountActor>(Method::AuthenticateMessage as MethodNum, &params),
+    );
+
+    // Ok to call exported method number
+    rt.expect_validate_caller_any();
+    rt.expect_verify_signature(ExpectedVerifySig {
+        sig: Signature::new_secp256k1(vec![]),
+        signer: addr,
+        plaintext: vec![],
+        result: Ok(()),
+    });
+    rt.call::<AccountActor>(Method::AuthenticateMessageExported as MethodNum, &params).unwrap();
+}
+
+fn check_state(rt: &MockRuntime) {
+    let test_address = Address::new_id(1000);
+    let (_, acc) = check_state_invariants(&rt.get_state(), &test_address);
+    acc.assert_empty();
 }
diff --git a/runtime/src/builtin/shared.rs b/runtime/src/builtin/shared.rs
@@ -3,8 +3,8 @@
 
 use crate::{actor_error, ActorContext, ActorError};
 use fvm_shared::address::Address;
-use fvm_shared::ActorID;
 use fvm_shared::METHOD_SEND;
+use fvm_shared::{ActorID, MethodNum};
 
 use crate::runtime::builtins::Type;
 use crate::runtime::Runtime;
@@ -38,3 +38,37 @@ pub fn resolve_to_actor_id(
 
     Err(actor_error!(illegal_argument, "failed to resolve or initialize address {}", address))
 }
+
+// The lowest FRC-42 method number.
+pub const FIRST_EXPORTED_METHOD_NUMBER: MethodNum = 1 << 24;
+
+// Checks whether the caller is allowed to invoke some method number.
+// All method numbers below the FRC-42 range are restricted to built-in actors
+// (including the account and multisig actors).
+// Methods may subsequently enforce tighter restrictions.
+pub fn restrict_internal_api<RT>(rt: &mut RT, method: MethodNum) -> Result<(), ActorError>
+where
+    RT: Runtime,
+{
+    if method >= FIRST_EXPORTED_METHOD_NUMBER {
+        return Ok(());
+    }
+    let caller = rt.message().caller();
+    let code_cid = rt.get_actor_code_cid(&caller.id().unwrap());
+    match code_cid {
+        None => {
+            return Err(
+                actor_error!(forbidden; "no code for caller {} of method {}", caller, method),
+            )
+        }
+        Some(code_cid) => {
+            let builtin_type = rt.resolve_builtin_actor_type(&code_cid);
+            if builtin_type.is_none() {
+                return Err(
+                    actor_error!(forbidden; "caller {} of method {} must be built-in", caller, method),
+                );
+            }
+        }
+    }
+    Ok(())
+}
diff --git a/runtime/src/test_utils.rs b/runtime/src/test_utils.rs
@@ -42,18 +42,18 @@ use crate::runtime::{
 use crate::{actor_error, ActorError};
 
 lazy_static::lazy_static! {
-    pub static ref SYSTEM_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/system");
-    pub static ref INIT_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/init");
-    pub static ref CRON_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/cron");
-    pub static ref ACCOUNT_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/account");
-    pub static ref POWER_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/storagepower");
-    pub static ref MINER_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/storageminer");
-    pub static ref MARKET_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/storagemarket");
-    pub static ref PAYCH_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/paymentchannel");
-    pub static ref MULTISIG_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/multisig");
-    pub static ref REWARD_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/reward");
-    pub static ref VERIFREG_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/verifiedregistry");
-    pub static ref DATACAP_TOKEN_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/datacap");
+    pub static ref SYSTEM_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/system");
+    pub static ref INIT_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/init");
+    pub static ref CRON_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/cron");
+    pub static ref ACCOUNT_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/account");
+    pub static ref POWER_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/storagepower");
+    pub static ref MINER_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/storageminer");
+    pub static ref MARKET_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/storagemarket");
+    pub static ref PAYCH_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/paymentchannel");
+    pub static ref MULTISIG_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/multisig");
+    pub static ref REWARD_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/reward");
+    pub static ref VERIFREG_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/verifiedregistry");
+    pub static ref DATACAP_TOKEN_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/datacap");
     pub static ref ACTOR_TYPES: BTreeMap<Cid, Type> = {
         let mut map = BTreeMap::new();
         map.insert(*SYSTEM_ACTOR_CODE_ID, Type::System);
@@ -99,7 +99,7 @@ lazy_static::lazy_static! {
 const IPLD_RAW: u64 = 0x55;
 
 /// Returns an identity CID for bz.
-pub fn make_builtin(bz: &[u8]) -> Cid {
+pub fn make_identity_cid(bz: &[u8]) -> Cid {
     Cid::new_v1(IPLD_RAW, OtherMultihash::wrap(0, bz).expect("name too long"))
 }
 
diff --git a/test_vm/tests/test_vm_test.rs b/test_vm/tests/test_vm_test.rs
@@ -1,5 +1,7 @@
 use fil_actor_account::State as AccountState;
-use fil_actors_runtime::test_utils::{make_builtin, ACCOUNT_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID};
+use fil_actors_runtime::test_utils::{
+    make_identity_cid, ACCOUNT_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID,
+};
 use fvm_ipld_blockstore::MemoryBlockstore;
 use fvm_ipld_encoding::RawBytes;
 use fvm_shared::address::Address;
@@ -18,14 +20,19 @@ fn state_control() {
     let addr2 = Address::new_id(2222);
 
     // set actor
-    let a1 =
-        actor(*ACCOUNT_ACTOR_CODE_ID, make_builtin(b"a1-head"), 42, TokenAmount::from_atto(10u8));
+    let a1 = actor(
+        *ACCOUNT_ACTOR_CODE_ID,
+        make_identity_cid(b"a1-head"),
+        42,
+        TokenAmount::from_atto(10u8),
+    );
     v.set_actor(addr1, a1.clone());
     let out = v.get_actor(addr1).unwrap();
     assert_eq!(out, a1);
     let check = v.checkpoint();
 
-    let a2 = actor(*PAYCH_ACTOR_CODE_ID, make_builtin(b"a2-head"), 88, TokenAmount::from_atto(1u8));
+    let a2 =
+        actor(*PAYCH_ACTOR_CODE_ID, make_identity_cid(b"a2-head"), 88, TokenAmount::from_atto(1u8));
     v.set_actor(addr2, a2.clone());
     assert_eq!(v.get_actor(addr2).unwrap(), a2);
     // rollback removes a2 but not a1
