backport: fix: notaries cannot remove datacap from verifreg (#784) (#785

) fix: notaries cannot remove datacap from verifreg (#784) * remove_verified_client_data_cap cannot remove datacap from verifreg actor * Verifreg test: fail to remove datacap of the verifreg itself * Verifreg: fix error message Co-authored-by: Aayush <[email protected]> Co-authored-by: Geoff Stuart <[email protected]>
filecoin-project · Oct 26, 2022 · c88d9ff · c88d9ff
1 parent f22674a
commit c88d9ff
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 4 deletions.
diff --git a/actors/verifreg/src/lib.rs b/actors/verifreg/src/lib.rs
@@ -24,7 +24,7 @@ use fil_actors_runtime::runtime::{ActorCode, Policy, Runtime};
 use fil_actors_runtime::{
     actor_error, cbor, make_map_with_root_and_bitwidth, resolve_to_actor_id, ActorDowncast,
     ActorError, BatchReturn, Map, DATACAP_TOKEN_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR,
-    SYSTEM_ACTOR_ADDR,
+    SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
 };
 use fil_actors_runtime::{ActorContext, AsActorError, BatchReturnGen};
 
@@ -243,6 +243,13 @@ impl Actor {
         rt.transaction(|st: &mut State, rt| {
             rt.validate_immediate_caller_is(std::iter::once(&st.root_key))?;
 
+            if params.verified_client_to_remove == VERIFIED_REGISTRY_ACTOR_ADDR {
+                return Err(actor_error!(
+                    illegal_argument,
+                    "cannot remove data cap from verified registry itself"
+                ));
+            }
+
             if !is_verifier(rt, st, verifier_1)? {
                 return Err(actor_error!(not_found, "{} is not a verifier", verifier_1));
             }

diff --git a/test_vm/tests/verifreg_remove_datacap_test.rs b/test_vm/tests/verifreg_remove_datacap_test.rs
@@ -15,8 +15,8 @@ use fil_actor_datacap::{
     DestroyParams, Method as DataCapMethod, MintParams, State as DataCapState,
 };
 use fil_actor_verifreg::{
-    AddVerifierClientParams, RemoveDataCapParams, RemoveDataCapRequest, RemoveDataCapReturn,
-    SIGNATURE_DOMAIN_SEPARATION_REMOVE_DATA_CAP,
+    AddVerifierClientParams, DataCap, RemoveDataCapParams, RemoveDataCapRequest,
+    RemoveDataCapReturn, SIGNATURE_DOMAIN_SEPARATION_REMOVE_DATA_CAP,
 };
 use fil_actor_verifreg::{AddrPairKey, Method as VerifregMethod};
 use fil_actor_verifreg::{RemoveDataCapProposal, RemoveDataCapProposalID, State as VerifregState};
@@ -25,7 +25,7 @@ use fil_actors_runtime::{
     make_map_with_root_and_bitwidth, DATACAP_TOKEN_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR,
     VERIFIED_REGISTRY_ACTOR_ADDR,
 };
-use test_vm::util::{apply_ok, create_accounts, verifreg_add_verifier};
+use test_vm::util::{apply_code, apply_ok, create_accounts, verifreg_add_verifier};
 use test_vm::{ExpectInvocation, TEST_VERIFREG_ROOT_ADDR, VM};
 
 #[test]
@@ -267,6 +267,63 @@ fn remove_datacap_simple_successful_path() {
     assert_eq!(2u64, verifier2_proposal_id.id);
     v.assert_state_invariants();
 }
+#[test]
+fn remove_datacap_fails_on_verifreg() {
+    let store = MemoryBlockstore::new();
+    let v = VM::new_with_singletons(&store);
+    let addrs = create_accounts(&v, 2, TokenAmount::from_whole(10_000));
+    let (verifier1, verifier2) = (addrs[0], addrs[1]);
+
+    let verifier1_id_addr = v.normalize_address(&verifier1).unwrap();
+    let verifier2_id_addr = v.normalize_address(&verifier2).unwrap();
+    let verifier_allowance = StoragePower::from(2 * 1048576u64);
+    let allowance_to_remove: StoragePower = DataCap::from(100);
+
+    // register verifier1 and verifier2
+    verifreg_add_verifier(&v, verifier1, verifier_allowance.clone());
+    verifreg_add_verifier(&v, verifier2, verifier_allowance);
+
+    let remove_proposal = RemoveDataCapProposal {
+        verified_client: VERIFIED_REGISTRY_ACTOR_ADDR,
+        data_cap_amount: allowance_to_remove.clone(),
+        removal_proposal_id: RemoveDataCapProposalID { id: 0 },
+    };
+
+    let mut remove_proposal_ser = to_vec(&remove_proposal).unwrap();
+    let mut remove_proposal_payload = SIGNATURE_DOMAIN_SEPARATION_REMOVE_DATA_CAP.to_vec();
+    remove_proposal_payload.append(&mut remove_proposal_ser);
+
+    let remove_datacap_params = RemoveDataCapParams {
+        verified_client_to_remove: VERIFIED_REGISTRY_ACTOR_ADDR,
+        data_cap_amount_to_remove: allowance_to_remove,
+        verifier_request_1: RemoveDataCapRequest {
+            verifier: verifier1_id_addr,
+            signature: Signature {
+                sig_type: SignatureType::Secp256k1,
+                bytes: remove_proposal_payload.clone(),
+            },
+        },
+        verifier_request_2: RemoveDataCapRequest {
+            verifier: verifier2_id_addr,
+            signature: Signature {
+                sig_type: SignatureType::Secp256k1,
+                bytes: remove_proposal_payload,
+            },
+        },
+    };
+
+    apply_code(
+        &v,
+        TEST_VERIFREG_ROOT_ADDR,
+        VERIFIED_REGISTRY_ACTOR_ADDR,
+        TokenAmount::zero(),
+        VerifregMethod::RemoveVerifiedClientDataCap as u64,
+        remove_datacap_params,
+        ExitCode::USR_ILLEGAL_ARGUMENT,
+    );
+
+    v.assert_state_invariants();
+}
 
 fn expect_remove_datacap(params: &RemoveDataCapParams) -> ExpectInvocation {
     ExpectInvocation {