Skip to content

Commit

Permalink
fix: 🩹 Consider providers awaiting for top up as insolvent
Browse files Browse the repository at this point in the history
  • Loading branch information
@ffarall
ffarall committed Dec 17, 2024
1 parent 6f9b06d commit 0f4d34a
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 131 deletions.
2 changes: 1 addition & 1 deletion pallets/payment-streams/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub mod pallet {
+ hold::Mutate<Self::AccountId, Reason = Self::RuntimeHoldReason>;

/// The trait for reading provider data.
type ProvidersPallet: ReadProvidersInterface<AccountId = Self::AccountId, TickNumber = BlockNumberFor<Self>>
type ProvidersPallet: ReadProvidersInterface<AccountId = Self::AccountId>
+ SystemMetricsInterface<ProvidedUnit = Self::Units>;

/// The trait exposing data of which providers submitted valid proofs in which ticks
Expand Down
137 changes: 38 additions & 99 deletions pallets/payment-streams/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,7 @@ where

// Charge the payment stream with the old rate before updating it to prevent abuse
let (amount_charged, last_tick_charged) =
match Self::do_charge_payment_streams(&provider_id, user_account) {
Ok((amount_charged, last_tick_charged)) => (amount_charged, last_tick_charged),
Err(err) if err == DispatchError::from(Error::<T>::ProviderInsolvent) => {
// If the provider is insolvent, we don't charge the user
(Zero::zero(), OnPollTicker::<T>::get())
}
Err(err) => return Err(err),
};
Self::do_charge_payment_streams(&provider_id, user_account)?;
if amount_charged > Zero::zero() {
let charged_at_tick = Self::get_current_tick();

Expand Down Expand Up @@ -260,28 +253,23 @@ where
Error::<T>::PaymentStreamNotFound
);

// Charge the user if the provider is not insolvent
// Charge the payment stream before deletion to make sure the services provided by the Provider is paid in full for its duration
let (amount_charged, last_tick_charged) =
match Self::do_charge_payment_streams(&provider_id, user_account) {
Ok((amount_charged, last_tick_charged)) => (amount_charged, last_tick_charged),
Err(err) if err == DispatchError::from(Error::<T>::ProviderInsolvent) => {
// If the provider is insolvent, we don't charge the user
(Zero::zero(), OnPollTicker::<T>::get())
}
Err(err) => return Err(err),
};
if amount_charged > Zero::zero() {
let charged_at_tick = Self::get_current_tick();
// We only charge if the Provider is solvent
if !<T::ProvidersPallet as ReadProvidersInterface>::is_provider_insolvent(*provider_id) {
let (amount_charged, last_tick_charged) =
Self::do_charge_payment_streams(&provider_id, user_account)?;
if amount_charged > Zero::zero() {
let charged_at_tick = Self::get_current_tick();

// We emit a payment charged event only if the user had to pay before being able to delete the payment stream
Self::deposit_event(Event::<T>::PaymentStreamCharged {
user_account: user_account.clone(),
provider_id: *provider_id,
amount: amount_charged,
last_tick_charged,
charged_at_tick,
});
// We emit a payment charged event only if the user had to pay before being able to delete the payment stream
Self::deposit_event(Event::<T>::PaymentStreamCharged {
user_account: user_account.clone(),
provider_id: *provider_id,
amount: amount_charged,
last_tick_charged,
charged_at_tick,
});
}
}

// The payment stream may have been deleted when charged if the user was out of funds.
Expand Down Expand Up @@ -472,14 +460,7 @@ where

// Charge the payment stream with the old amount before updating it to prevent abuse
let (amount_charged, last_tick_charged) =
match Self::do_charge_payment_streams(&provider_id, user_account) {
Ok((amount_charged, last_tick_charged)) => (amount_charged, last_tick_charged),
Err(err) if err == DispatchError::from(Error::<T>::ProviderInsolvent) => {
// If the provider is insolvent, we don't charge the user
(Zero::zero(), OnPollTicker::<T>::get())
}
Err(err) => return Err(err),
};
Self::do_charge_payment_streams(&provider_id, user_account)?;
if amount_charged > Zero::zero() {
let charged_at_tick = Self::get_current_tick();

Expand Down Expand Up @@ -540,28 +521,23 @@ where
Error::<T>::PaymentStreamNotFound
);

// Charge the payment stream if the provider is not insolvent
// Charge the payment stream before deletion to make sure the services provided by the Provider is paid in full for its duration
let (amount_charged, last_tick_charged) =
match Self::do_charge_payment_streams(&provider_id, user_account) {
Ok((amount_charged, last_tick_charged)) => (amount_charged, last_tick_charged),
Err(err) if err == DispatchError::from(Error::<T>::ProviderInsolvent) => {
// If the provider is insolvent, we don't charge the user
(Zero::zero(), OnPollTicker::<T>::get())
}
Err(err) => return Err(err),
};
if amount_charged > Zero::zero() {
let charged_at_tick = Self::get_current_tick();
// We only charge if the Provider is solvent
if !<T::ProvidersPallet as ReadProvidersInterface>::is_provider_insolvent(*provider_id) {
let (amount_charged, last_tick_charged) =
Self::do_charge_payment_streams(&provider_id, user_account)?;
if amount_charged > Zero::zero() {
let charged_at_tick = Self::get_current_tick();

// We emit a payment charged event only if the user had to pay before being able to delete the payment stream
Self::deposit_event(Event::<T>::PaymentStreamCharged {
user_account: user_account.clone(),
provider_id: *provider_id,
amount: amount_charged,
last_tick_charged,
charged_at_tick,
});
// We emit a payment charged event only if the user had to pay before being able to delete the payment stream
Self::deposit_event(Event::<T>::PaymentStreamCharged {
user_account: user_account.clone(),
provider_id: *provider_id,
amount: amount_charged,
last_tick_charged,
charged_at_tick,
});
}
}

// The payment stream may have been deleted when charged if the user was out of funds.
Expand Down Expand Up @@ -654,7 +630,7 @@ where
}
None => {
// If the user hasn't been flagged as without funds, charge the payment stream
// Calculate the time passed between the adjusted last chargeable tick and the last charged tick
// Calculate the time passed between the last chargeable tick and the last charged tick
if let Some(time_passed) = last_chargeable_tick
.checked_sub(&fixed_rate_payment_stream.last_charged_tick)
{
Expand Down Expand Up @@ -771,7 +747,7 @@ where
Preservation::Preserve,
)?;

// Set the last charged tick to the adjusted last chargeable tick
// Set the last charged tick to the tick number of the last chargeable tick
FixedRatePaymentStreams::<T>::mutate(
provider_id,
user_account,
Expand Down Expand Up @@ -927,7 +903,7 @@ where
Preservation::Preserve,
)?;

// Set the last charged price index to be the adjusted price index at the last chargeable tick
// Set the last charged price index to be the price index of the last chargeable tick
DynamicRatePaymentStreams::<T>::mutate(
provider_id,
user_account,
Expand Down Expand Up @@ -969,14 +945,7 @@ where
// and emit a PaymentStreamCharged event if the User had to pay.
for user_account in user_accounts.iter() {
let (amount_charged, last_tick_charged) =
match Self::do_charge_payment_streams(&provider_id, user_account) {
Ok((amount_charged, last_tick_charged)) => (amount_charged, last_tick_charged),
Err(err) if err == DispatchError::from(Error::<T>::ProviderInsolvent) => {
// If the provider is insolvent, we don't charge the user
(Zero::zero(), OnPollTicker::<T>::get())
}
Err(err) => return Err(err),
};
Self::do_charge_payment_streams(provider_id, user_account)?;

if amount_charged > Zero::zero() {
Self::deposit_event(Event::<T>::PaymentStreamCharged {
Expand Down Expand Up @@ -1261,13 +1230,6 @@ where
if let Some(proof_submitters_to_process) = maybe_proof_submitters {
// Update the last chargeable info of all Providers that submitted a valid proof in the tick to process.
for provider_id in &proof_submitters_to_process {
// Skip if provider is insolvent
if <T::ProvidersPallet as ReadProvidersInterface>::is_provider_insolvent(
*provider_id,
) {
continue;
}

// Update the last chargeable tick and last chargeable price index of the Provider.
// The last chargeable tick is set to the current tick of THIS PALLET. That means that if the tick from
// the pallet that implements the `ProofSubmittersInterface` trait is stalled for some time (and this pallet
Expand Down Expand Up @@ -1861,41 +1823,18 @@ where
providers
}

/// Returns the `ProviderLastChargeableInfo` of a Provider, which includes the last chargeable tick and the last chargeable price index.
///
/// If the provider is insolvent, the last chargeable tick is set to the block at which the provider became insolvent since we do not allow
/// providers to charge for the time they were insolvent. `LastChargeableInfo` is updated to reflect this change.
/// Returns the [`ProviderLastChargeableInfo`] of a Provider, which includes the last chargeable tick and the last chargeable price index.
pub fn get_last_chargeable_info_with_privilege(
provider_id: &ProviderIdFor<T>,
) -> ProviderLastChargeableInfo<T> {
let current_tick = Self::get_current_tick();

// Get the block number at which the provider cannot charge anymore (if any)
let maybe_non_chargeable_tick =
<T::ProvidersPallet as ReadProvidersInterface>::starting_non_chargeable_tick(
*provider_id,
);

// Adjust the last chargeable tick to exclude the time when the provider cannot charge
let adjusted_last_chargeable_tick = match maybe_non_chargeable_tick {
Some(non_chargeable_tick) => current_tick.min(non_chargeable_tick),
None => current_tick,
};

// If this is a Privileged Provider, then it is allowed to charge up to the current tick.
if let Some(_) = PrivilegedProviders::<T>::get(provider_id) {
return ProviderLastChargeableInfo {
last_chargeable_tick: adjusted_last_chargeable_tick,
last_chargeable_tick: Self::get_current_tick(),
price_index: Default::default(),
};
}

if current_tick != adjusted_last_chargeable_tick {
LastChargeableInfo::<T>::mutate(provider_id, |info| {
info.last_chargeable_tick = adjusted_last_chargeable_tick;
});
}

return LastChargeableInfo::<T>::get(provider_id);
}
}
2 changes: 0 additions & 2 deletions pallets/providers/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ pub struct TopUpMetadata<T: Config> {
///
/// This is used for payment streams to determine when the provider should not be able to charge the user anymore starting
/// from this tick number.
///
/// It is referenced in the [`starting_non_chargeable_tick`](shp_traits::ReadProvidersInterface::starting_non_chargeable_tick) trait implementation.
pub started_at: PaymentStreamsTickNumber<T>,
/// The Storage Hub tick number at which the provider will be marked as insolvent.
///
Expand Down
40 changes: 20 additions & 20 deletions pallets/providers/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ use sp_runtime::traits::ConvertBack;
use sp_std::vec::Vec;
use types::{
Bucket, Commitment, ExpirationItem, MainStorageProvider, MainStorageProviderSignUpRequest,
MultiAddress, Multiaddresses, PaymentStreamsTickNumber, ProviderIdFor, RateDeltaParam,
SignUpRequestSpParams, StorageDataUnitAndBalanceConverter, StorageProviderId, TopUpMetadata,
ValuePropIdFor, ValueProposition, ValuePropositionWithId,
MultiAddress, Multiaddresses, ProviderIdFor, RateDeltaParam, SignUpRequestSpParams,
StorageDataUnitAndBalanceConverter, StorageProviderId, TopUpMetadata, ValuePropIdFor,
ValueProposition, ValuePropositionWithId,
};

macro_rules! expect_or_err {
Expand Down Expand Up @@ -2061,7 +2061,6 @@ impl<T: pallet::Config> ReadProvidersInterface for pallet::Pallet<T> {
type Balance = T::NativeBalance;
type MerkleHash = MerklePatriciaRoot<T>;
type ProviderId = ProviderIdFor<T>;
type TickNumber = PaymentStreamsTickNumber<T>;

fn get_default_root() -> Self::MerkleHash {
T::DefaultMerkleRoot::get()
Expand Down Expand Up @@ -2141,24 +2140,25 @@ impl<T: pallet::Config> ReadProvidersInterface for pallet::Pallet<T> {
}

fn is_provider_insolvent(who: Self::ProviderId) -> bool {
InsolventProviders::<T>::get(&StorageProviderId::<T>::MainStorageProvider(who)).is_some()
|| InsolventProviders::<T>::get(&StorageProviderId::<T>::BackupStorageProvider(who))
let is_provider_insolvent =
InsolventProviders::<T>::get(&StorageProviderId::<T>::MainStorageProvider(who))
.is_some()
}
|| InsolventProviders::<T>::get(&StorageProviderId::<T>::BackupStorageProvider(
who,
))
.is_some();

// While provider is being awaited for top up, it is still considered insolvent, it's just that
// it can get out of this state.
let is_provider_awaiting_topup =
AwaitingTopUpFromProviders::<T>::get(&StorageProviderId::<T>::MainStorageProvider(who))
.is_some()
|| AwaitingTopUpFromProviders::<T>::get(
&StorageProviderId::<T>::BackupStorageProvider(who),
)
.is_some();

fn starting_non_chargeable_tick(who: Self::ProviderId) -> Option<Self::TickNumber> {
// Providers cannot charge for the time they are in the state of being awaited for to top up their deposit.
if let Some(top_up_metadata) =
AwaitingTopUpFromProviders::<T>::get(StorageProviderId::<T>::MainStorageProvider(who))
{
Some(top_up_metadata.started_at)
} else if let Some(top_up_metadata) =
AwaitingTopUpFromProviders::<T>::get(StorageProviderId::<T>::BackupStorageProvider(who))
{
Some(top_up_metadata.started_at)
} else {
None
}
is_provider_insolvent || is_provider_awaiting_topup
}
}

Expand Down
9 changes: 0 additions & 9 deletions primitives/traits/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,11 +599,6 @@ pub trait ReadProvidersInterface {
+ MaxEncodedLen
+ FullCodec;

/// The type which represents ticks.
///
/// Used to keep track of the system time.
type TickNumber: Parameter + Member + MaybeSerializeDeserialize + Debug + Ord + MaxEncodedLen;

/// The Balance type of the runtime, which should correspond to the type of
/// the staking balance of a registered Provider.
type Balance: fungible::Inspect<Self::AccountId> + fungible::hold::Inspect<Self::AccountId>;
Expand Down Expand Up @@ -633,10 +628,6 @@ pub trait ReadProvidersInterface {

/// Check if the provider is insolvent.
fn is_provider_insolvent(who: Self::ProviderId) -> bool;

/// Potentially non-chargeable tick for a provider. From this tick onwards, the provider cannot charge for
/// the storage services.
fn starting_non_chargeable_tick(who: Self::ProviderId) -> Option<Self::TickNumber>;
}

/// A trait to mutate the state of a generic Provider, such as updating their root.
Expand Down

0 comments on commit 0f4d34a

Please sign in to comment.