Skip to content
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.

Sign up for GitHub

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

Delete deprecated UseBytes and RestoreBytes methods from verified registry #658

Merged
merged 1 commit into from
Sep 16, 2022
Merged

Delete deprecated UseBytes and RestoreBytes methods from verified registry #658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
99 changes: 2 additions & 97 deletions actors/verifreg/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ pub enum Method {
AddVerifier = 2,
RemoveVerifier = 3,
AddVerifiedClient = 4,
UseBytes = 5, // Deprecated
RestoreBytes = 6, // Deprecated
// UseBytes = 5, // Deprecated
// RestoreBytes = 6, // Deprecated
RemoveVerifiedClientDataCap = 7,
RemoveExpiredAllocations = 8,
ClaimAllocations = 9,
Expand Down Expand Up @@ -224,93 +224,6 @@ impl Actor {
Ok(())
}

/// Called by StorageMarketActor during PublishStorageDeals.
/// Do not allow partially verified deals (DealSize must be greater than equal to allowed cap).
/// Delete VerifiedClient if remaining DataCap is smaller than minimum VerifiedDealSize.
pub fn use_bytes<BS, RT>(rt: &mut RT, params: UseBytesParams) -> Result<(), ActorError>
where
BS: Blockstore,
RT: Runtime<BS>,
{
rt.validate_immediate_caller_is(std::iter::once(&*STORAGE_MARKET_ACTOR_ADDR))?;

let client = resolve_to_actor_id(rt, &params.address).context_code(
ExitCode::USR_ILLEGAL_STATE,
format!("failed to resolve addr {} to ID addr", params.address),
)?;

let client = Address::new_id(client);

if params.deal_size < rt.policy().minimum_verified_allocation_size {
return Err(actor_error!(
illegal_argument,
"use bytes {} is below minimum {}",
params.deal_size,
rt.policy().minimum_verified_allocation_size
));
}

// Deduct from client's token allowance.
let remaining = destroy(rt, &client, &params.deal_size).context(format!(
"failed to deduct {} from allowance for {}",
&params.deal_size, &client
))?;

// Destroy any remaining balance below minimum verified deal size.
if remaining.is_positive() && remaining < rt.policy().minimum_verified_allocation_size {
destroy(rt, &client, &remaining).context(format!(
"failed to destroy remaining {} from allowance for {}",
&remaining, &client
))?;
}
Ok(())
}

/// Called by HandleInitTimeoutDeals from StorageMarketActor when a VerifiedDeal fails to init.
/// Restore allowable cap for the client, creating new entry if the client has been deleted.
pub fn restore_bytes<BS, RT>(rt: &mut RT, params: RestoreBytesParams) -> Result<(), ActorError>
where
BS: Blockstore,
RT: Runtime<BS>,
{
rt.validate_immediate_caller_is(std::iter::once(&*STORAGE_MARKET_ACTOR_ADDR))?;
if params.deal_size < rt.policy().minimum_verified_allocation_size {
return Err(actor_error!(
illegal_argument,
"Below minimum VerifiedDealSize requested in RestoreBytes: {}",
params.deal_size
));
}

let client = resolve_to_actor_id(rt, &params.address).context_code(
ExitCode::USR_ILLEGAL_STATE,
format!("failed to resolve addr {} to ID addr", params.address),
)?;

let client = Address::new_id(client);

let st: State = rt.state()?;
// Disallow root as a client.
if client == st.root_key {
return Err(actor_error!(illegal_argument, "cannot restore allowance for root"));
}

// Disallow existing verifiers as clients.
if st.get_verifier_cap(rt.store(), &client)?.is_some() {
return Err(actor_error!(
illegal_argument,
"cannot restore allowance for verifier {}",
client
));
}

let operators = vec![*STORAGE_MARKET_ACTOR_ADDR];
mint(rt, &client, &params.deal_size, operators).context(format!(
"failed to restore {} to allowance for {}",
&params.deal_size, &client
))
}

/// Removes DataCap allocated to a verified client.
pub fn remove_verified_client_data_cap<BS, RT>(
rt: &mut RT,
Expand Down Expand Up @@ -1054,14 +967,6 @@ impl ActorCode for Actor {
Self::add_verified_client(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::default())
}
Some(Method::UseBytes) => {
Self::use_bytes(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::default())
}
Some(Method::RestoreBytes) => {
Self::restore_bytes(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::default())
}
Some(Method::RemoveVerifiedClientDataCap) => {
let res =
Self::remove_verified_client_data_cap(rt, cbor::deserialize_params(params)?)?;
Expand Down
12 changes: 0 additions & 12 deletions actors/verifreg/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,6 @@ pub type AddVerifierClientParams = VerifierParams;
/// We can introduce policy changes and replace this in the future.
pub type DataCap = StoragePower;

#[derive(Clone, Debug, PartialEq, Eq, Serialize_tuple, Deserialize_tuple)]
pub struct BytesParams {
/// Address of verified client.
pub address: Address,
/// Number of bytes to use.
#[serde(with = "bigint_ser")]
pub deal_size: StoragePower,
}

pub type UseBytesParams = BytesParams;
pub type RestoreBytesParams = BytesParams;

pub const SIGNATURE_DOMAIN_SEPARATION_REMOVE_DATA_CAP: &[u8] = b"fil_removedatacap:";

impl Cbor for RemoveDataCapParams {}
Expand Down
37 changes: 1 addition & 36 deletions actors/verifreg/tests/harness/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use fil_actor_verifreg::{
AllocationID, AllocationRequest, AllocationRequests, AllocationsResponse,
ClaimAllocationsParams, ClaimAllocationsReturn, ClaimID, DataCap, GetClaimsParams,
GetClaimsReturn, Method, RemoveExpiredAllocationsParams, RemoveExpiredAllocationsReturn,
RestoreBytesParams, SectorAllocationClaim, State,
SectorAllocationClaim, State,
};
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::runtime::policy_constants::{
Expand Down Expand Up @@ -200,41 +200,6 @@ impl Harness {
Ok(())
}

pub fn restore_bytes(
&self,
rt: &mut MockRuntime,
client: &Address,
amount: &DataCap,
) -> Result<(), ActorError> {
rt.expect_validate_caller_addr(vec![*STORAGE_MARKET_ACTOR_ADDR]);
rt.set_caller(*MARKET_ACTOR_CODE_ID, *STORAGE_MARKET_ACTOR_ADDR);
let client_resolved = rt.get_id_address(client).unwrap_or(*client);

// Expect tokens to be minted.
let mint_params = ext::datacap::MintParams {
to: client_resolved,
amount: TokenAmount::from_whole(amount.to_i64().unwrap()),
operators: vec![*STORAGE_MARKET_ACTOR_ADDR],
};
rt.expect_send(
*DATACAP_TOKEN_ACTOR_ADDR,
ext::datacap::Method::Mint as MethodNum,
RawBytes::serialize(&mint_params).unwrap(),
TokenAmount::zero(),
RawBytes::default(),
ExitCode::OK,
);

let params = RestoreBytesParams { address: *client, deal_size: amount.clone() };
let ret = rt.call::<VerifregActor>(
Method::RestoreBytes as MethodNum,
&RawBytes::serialize(params).unwrap(),
)?;
assert_eq!(RawBytes::default(), ret);
rt.verify();
Ok(())
}

pub fn check_state(&self, rt: &MockRuntime) {
let (_, acc) = check_state_invariants(&rt.get_state(), rt.store());
acc.assert_empty();
Expand Down
80 changes: 2 additions & 78 deletions actors/verifreg/tests/verifreg_actor_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,20 +552,16 @@ mod datacap {
use fvm_shared::error::ExitCode;
use fvm_shared::{ActorID, MethodNum};

use fil_actor_verifreg::{Actor as VerifregActor, Method, RestoreBytesParams, State};
use fil_actor_verifreg::{Actor as VerifregActor, Method, State};
use fil_actors_runtime::cbor::serialize;
use fil_actors_runtime::runtime::policy_constants::{
MAXIMUM_VERIFIED_ALLOCATION_EXPIRATION, MAXIMUM_VERIFIED_ALLOCATION_TERM,
MINIMUM_VERIFIED_ALLOCATION_SIZE, MINIMUM_VERIFIED_ALLOCATION_TERM,
};
use fil_actors_runtime::runtime::Runtime;
use fil_actors_runtime::test_utils::*;
use fil_actors_runtime::{
DATACAP_TOKEN_ACTOR_ADDR, EPOCHS_IN_YEAR, STORAGE_MARKET_ACTOR_ADDR,
STORAGE_POWER_ACTOR_ADDR,
};
use fil_actors_runtime::{DATACAP_TOKEN_ACTOR_ADDR, EPOCHS_IN_YEAR, STORAGE_MARKET_ACTOR_ADDR};
use harness::*;
use util::*;

use crate::*;

Expand Down Expand Up @@ -790,76 +786,4 @@ mod datacap {
}
h.check_state(&rt);
}

#[test]
fn restore() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a feeling we'll want to reinstate these tests for verifreg::RemoveExpiredAllocations. A follow up is fine but in that case maybe leave these tests commented out so review is easier?

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked through and I think they'll be different enough to need basically re-writing. It's a good point to come check here for reference though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking through these now while finishing off testing:

  • We use actor IDs for the client/provider, so no longer need to resolve addresses
  • Anyone can call RemoveExpiredAllocations, not just the market actor
  • No need to check minimums since we're dealing with allocation records the verifreg already has

let (h, mut rt) = new_harness();
let deal_size = &rt.policy.minimum_verified_allocation_size.clone();
h.restore_bytes(&mut rt, &CLIENT, deal_size).unwrap();
h.check_state(&rt);
}

#[test]
fn restore_resolves_client_address() {
let (h, mut rt) = new_harness();
let client_pubkey = Address::new_secp256k1(&[3u8; 65]).unwrap();
rt.id_addresses.insert(client_pubkey, *CLIENT);

// Restore to pubkey address.
let deal_size = rt.policy.minimum_verified_allocation_size.clone();
h.restore_bytes(&mut rt, &client_pubkey, &deal_size).unwrap();
h.check_state(&rt)
}

#[test]
fn restore_requires_market_actor_caller() {
let (h, mut rt) = new_harness();
rt.expect_validate_caller_addr(vec![*STORAGE_MARKET_ACTOR_ADDR]);
rt.set_caller(*POWER_ACTOR_CODE_ID, *STORAGE_POWER_ACTOR_ADDR);
let params = RestoreBytesParams {
address: *CLIENT,
deal_size: rt.policy.minimum_verified_allocation_size.clone(),
};
expect_abort(
ExitCode::USR_FORBIDDEN,
rt.call::<VerifregActor>(
Method::RestoreBytes as MethodNum,
&RawBytes::serialize(params).unwrap(),
),
);
h.check_state(&rt)
}

#[test]
fn restore_requires_minimum_deal_size() {
let (h, mut rt) = new_harness();

let deal_size = rt.policy.minimum_verified_allocation_size.clone() - 1;
expect_abort(ExitCode::USR_ILLEGAL_ARGUMENT, h.restore_bytes(&mut rt, &CLIENT, &deal_size));
h.check_state(&rt)
}

#[test]
fn restore_rejects_root() {
let (h, mut rt) = new_harness();
let deal_size = rt.policy.minimum_verified_allocation_size.clone();
expect_abort(
ExitCode::USR_ILLEGAL_ARGUMENT,
h.restore_bytes(&mut rt, &ROOT_ADDR, &deal_size),
);
h.check_state(&rt)
}

#[test]
fn restore_rejects_verifier() {
let (h, mut rt) = new_harness();
let allowance = verifier_allowance(&rt);
h.add_verifier(&mut rt, &VERIFIER, &allowance).unwrap();
let deal_size = rt.policy.minimum_verified_allocation_size.clone();
expect_abort(
ExitCode::USR_ILLEGAL_ARGUMENT,
h.restore_bytes(&mut rt, &VERIFIER, &deal_size),
);
h.check_state(&rt)
}
}