Skip to content

Commit

Permalink
Refactor on_container_author_noted hook to take a Vec
Browse files Browse the repository at this point in the history
  • Loading branch information
@tmpolaczyk
tmpolaczyk committed Dec 18, 2024
1 parent 14d6157 commit 104163e
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 133 deletions.
55 changes: 25 additions & 30 deletions pallets/author-noting/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,12 @@ use {
sp_consensus_aura::{inherents::InherentType, Slot, AURA_ENGINE_ID},
sp_inherents::{InherentIdentifier, IsFatalError},
sp_runtime::{traits::Header, DigestItem, DispatchResult, RuntimeString},
sp_std::vec::Vec,
tp_author_noting_inherent::INHERENT_IDENTIFIER,
tp_traits::{
AuthorNotingHook, ContainerChainBlockInfo, GenericStateProof, GenericStorageReader,
GetContainerChainAuthor, GetCurrentContainerChains, LatestAuthorInfoFetcher,
NativeStorageReader, ReadEntryErr,
AuthorNotingHook, AuthorNotingInfo, ContainerChainBlockInfo, GenericStateProof,
GenericStorageReader, GetContainerChainAuthor, GetCurrentContainerChains,
LatestAuthorInfoFetcher, NativeStorageReader, ReadEntryErr,
},
};

Expand Down Expand Up @@ -158,9 +159,8 @@ pub mod pallet {
let storage_reader = T::RelayOrPara::create_storage_reader(data);

let parent_tanssi_slot = u64::from(T::SlotBeacon::slot()).into();
let mut infos = Vec::with_capacity(registered_para_ids.len());

// TODO: we should probably fetch all authors-containers first
// then pass the vector to the hook, this would allow for a better estimation
for para_id in registered_para_ids {
match Self::fetch_block_info_from_proof(
&storage_reader,
Expand All @@ -173,31 +173,23 @@ pub mod pallet {
|maybe_old_block_info: &mut Option<
ContainerChainBlockInfo<T::AccountId>,
>| {
if let Some(ref mut old_block_info) = maybe_old_block_info {
if block_info.block_number > old_block_info.block_number {
// We only reward author if the block increases
total_weight = total_weight.saturating_add(
T::AuthorNotingHook::on_container_author_noted(
&block_info.author,
block_info.block_number,
para_id,
),
);
let _ = core::mem::replace(old_block_info, block_info);
}
} else {
// If there is no previous block, we should reward the author of the first block
total_weight = total_weight.saturating_add(
T::AuthorNotingHook::on_container_author_noted(
&block_info.author,
block_info.block_number,
para_id,
),
);
let _ = core::mem::replace(
maybe_old_block_info,
Some(block_info),
);
// No block number is the same as the last block number being 0:
// the first block created by collators is block number 1.
let old_block_number = maybe_old_block_info
.as_ref()
.map(|old_block_info| old_block_info.block_number)
.unwrap_or(0);
// We only reward author if the block increases
// If there is no previous block, we should reward the author of the first block
if block_info.block_number > old_block_number {
let bi = block_info.clone();
let info = AuthorNotingInfo {
author: block_info.author,
block_number: block_info.block_number,
para_id,
};
infos.push(info);
*maybe_old_block_info = Some(bi);
}
},
);
Expand All @@ -209,6 +201,9 @@ pub mod pallet {
),
}
}

total_weight = total_weight
.saturating_add(T::AuthorNotingHook::on_container_authors_noted(&infos));
}

// We correctly set the data
Expand Down
83 changes: 44 additions & 39 deletions pallets/inflation-rewards/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ mod mock;
#[cfg(test)]
mod tests;

use tp_traits::AuthorNotingInfo;
use {
dp_core::{BlockNumber, ParaId},
dp_core::ParaId,
frame_support::{
pallet_prelude::*,
traits::{
Expand Down Expand Up @@ -249,53 +250,57 @@ pub mod pallet {
// There will be no additional check other than checking if we have already
// rewarded this author for **in this tanssi block**
// Any additional check should be done in the calling function
// TODO: consider passing a vector here
impl<T: Config> AuthorNotingHook<T::AccountId> for Pallet<T> {
fn on_container_author_noted(
author: &T::AccountId,
_block_number: BlockNumber,
para_id: ParaId,
) -> Weight {
fn on_container_authors_noted(info: &[AuthorNotingInfo<T::AccountId>]) -> Weight {
let mut total_weight = T::DbWeight::get().reads_writes(1, 0);
// We take chains to reward, to see what containers are left to reward
if let Some(mut container_chains_to_reward) = ChainsToReward::<T>::get() {
// If we find the index is because we still have not rewarded it
if let Ok(index) = container_chains_to_reward.para_ids.binary_search(&para_id) {
// we distribute rewards to the author
match T::StakingRewardsDistributor::distribute_rewards(
author.clone(),
T::Currency::withdraw(
&T::PendingRewardsAccount::get(),
container_chains_to_reward.rewards_per_chain,
Precision::BestEffort,
Preservation::Expendable,
Fortitude::Force,
)
.unwrap_or(CreditOf::<T>::zero()),
) {
Ok(frame_support::dispatch::PostDispatchInfo { actual_weight, .. }) => {
Self::deposit_event(Event::RewardedContainer {
account_id: author.clone(),
balance: container_chains_to_reward.rewards_per_chain,
para_id,
});
if let Some(weight) = actual_weight {
total_weight += weight
for info in info {
let author = &info.author;
let para_id = info.para_id;

// If we find the index is because we still have not rewarded it
if let Ok(index) = container_chains_to_reward.para_ids.binary_search(&para_id) {
// we distribute rewards to the author
match T::StakingRewardsDistributor::distribute_rewards(
author.clone(),
T::Currency::withdraw(
&T::PendingRewardsAccount::get(),
container_chains_to_reward.rewards_per_chain,
Precision::BestEffort,
Preservation::Expendable,
Fortitude::Force,
)
.unwrap_or(CreditOf::<T>::zero()),
) {
Ok(frame_support::dispatch::PostDispatchInfo { actual_weight, .. }) => {
Self::deposit_event(Event::RewardedContainer {
account_id: author.clone(),
balance: container_chains_to_reward.rewards_per_chain,
para_id,
});
if let Some(weight) = actual_weight {
total_weight += weight
}
}
Err(e) => {
log::warn!("Fail to distribute rewards: {:?}", e)
}
}
Err(e) => {
log::debug!("Fail to distribute rewards: {:?}", e)
}
// we remove the para id from container-chains to reward
// this makes sure we dont reward it twice in the same block
container_chains_to_reward.para_ids.remove(index);
}
// we remove the para id from container-chains to reward
// this makes sure we dont reward it twice in the same block
container_chains_to_reward.para_ids.remove(index);

total_weight += T::DbWeight::get().writes(1);
// Keep track of chains to reward
ChainsToReward::<T>::put(container_chains_to_reward);
}

total_weight += T::DbWeight::get().writes(1);
// Keep track of chains to reward
ChainsToReward::<T>::put(container_chains_to_reward);
} else {
// TODO: why would ChainsToReward ever be None?
log::warn!("ChainsToReward is None");
}

total_weight
}

Expand Down
48 changes: 28 additions & 20 deletions pallets/inflation-rewards/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,13 @@ fn test_reward_container_chain_author() {

// Note container author
let registered_para_ids = <Test as Config>::ContainerChains::current_container_chains();
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_author_noted(
&container_author,
1,
registered_para_ids[0],
);
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_authors_noted(&[
AuthorNotingInfo {
author: container_author.clone(),
block_number: 1,
para_id: registered_para_ids[0],
},
]);

// Author should be rewarded immediately
assert_eq!(
Expand All @@ -148,11 +150,13 @@ fn test_reward_container_chain_author() {
let new_supply_2 = total_supply_2 - total_supply_1;

// Note next container author
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_author_noted(
&container_author_2,
2,
registered_para_ids[0],
);
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_authors_noted(&[
AuthorNotingInfo {
author: container_author_2.clone(),
block_number: 2,
para_id: registered_para_ids[0],
},
]);

// Author should be rewarded immediately
assert_eq!(
Expand All @@ -177,18 +181,22 @@ fn test_cannot_reward_twice_in_same_tanssi_block() {

// Note container author
let registered_para_ids = <Test as Config>::ContainerChains::current_container_chains();
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_author_noted(
&container_author,
1,
registered_para_ids[0],
);
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_authors_noted(&[
AuthorNotingInfo {
author: container_author.clone(),
block_number: 1,
para_id: registered_para_ids[0],
},
]);

// Regardless if we inject a new block, we cannot reward twice the same paraId
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_author_noted(
&container_author,
2,
registered_para_ids[0],
);
<Pallet<Test> as AuthorNotingHook<AccountId>>::on_container_authors_noted(&[
AuthorNotingInfo {
author: container_author.clone(),
block_number: 2,
para_id: registered_para_ids[0],
},
]);

// Author should be rewarded only once
assert_eq!(
Expand Down
44 changes: 23 additions & 21 deletions pallets/services-payment/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use {
serde::{Deserialize, Serialize},
sp_io::hashing::blake2_256,
sp_runtime::{traits::TrailingZeroInput, DispatchError},
tp_traits::{AuthorNotingHook, BlockNumber, CollatorAssignmentHook, CollatorAssignmentTip},
tp_traits::{AuthorNotingHook, CollatorAssignmentHook, CollatorAssignmentTip},
};

#[cfg(any(test, feature = "runtime-benchmarks"))]
Expand All @@ -50,6 +50,7 @@ pub mod weights;
pub use weights::WeightInfo;

pub use pallet::*;
use tp_traits::AuthorNotingInfo;

#[frame_support::pallet]
pub mod pallet {
Expand Down Expand Up @@ -540,31 +541,32 @@ impl<T: Config> AuthorNotingHook<T::AccountId> for Pallet<T> {
// This hook is called when pallet_author_noting sees that the block number of a container chain has increased.
// Currently we always charge 1 credit, even if a container chain produced more that 1 block in between tanssi
// blocks.
fn on_container_author_noted(
_author: &T::AccountId,
_block_number: BlockNumber,
para_id: ParaId,
) -> Weight {
if Pallet::<T>::burn_block_production_free_credit_for_para(&para_id).is_err() {
let (amount_to_charge, _weight) = T::ProvideBlockProductionCost::block_cost(&para_id);
fn on_container_authors_noted(info: &[AuthorNotingInfo<T::AccountId>]) -> Weight {
for info in info {
let para_id = info.para_id;
if Pallet::<T>::burn_block_production_free_credit_for_para(&para_id).is_err() {
let (amount_to_charge, _weight) =
T::ProvideBlockProductionCost::block_cost(&para_id);

match T::Currency::withdraw(
&Self::parachain_tank(para_id),
amount_to_charge,
WithdrawReasons::FEE,
ExistenceRequirement::KeepAlive,
) {
Err(e) => log::warn!(
"Failed to withdraw block production payment for container chain {}: {:?}",
u32::from(para_id),
e
),
Ok(imbalance) => {
T::OnChargeForBlock::on_unbalanced(imbalance);
match T::Currency::withdraw(
&Self::parachain_tank(para_id),
amount_to_charge,
WithdrawReasons::FEE,
ExistenceRequirement::KeepAlive,
) {
Err(e) => log::warn!(
"Failed to withdraw block production payment for container chain {}: {:?}",
u32::from(para_id),
e
),
Ok(imbalance) => {
T::OnChargeForBlock::on_unbalanced(imbalance);
}
}
}
}

// TODO: weight should depend on info.len()
T::WeightInfo::on_container_author_noted()
}

Expand Down
25 changes: 21 additions & 4 deletions pallets/services-payment/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Tanssi. If not, see <http://www.gnu.org/licenses/>

use tp_traits::AuthorNotingInfo;
use {
crate::{
mock::*, pallet as pallet_services_payment, BlockProductionCredits,
Expand Down Expand Up @@ -239,7 +240,11 @@ fn credits_should_be_substracted_from_tank_if_no_free_credits() {
1000u128
);

PaymentServices::on_container_author_noted(&1, 1, 1.into());
PaymentServices::on_container_authors_noted(&[AuthorNotingInfo {
author: 1,
block_number: 1,
para_id: 1.into(),
}]);

assert_eq!(
Balances::balance(&crate::Pallet::<Test>::parachain_tank(1.into())),
Expand All @@ -266,7 +271,11 @@ fn credits_should_not_be_substracted_from_tank_if_it_involves_death() {
100u128
);

PaymentServices::on_container_author_noted(&1, 1, 1.into());
PaymentServices::on_container_authors_noted(&[AuthorNotingInfo {
author: 1,
block_number: 1,
para_id: 1.into(),
}]);

assert_eq!(
Balances::balance(&crate::Pallet::<Test>::parachain_tank(1.into())),
Expand Down Expand Up @@ -303,7 +312,11 @@ fn not_having_enough_tokens_in_tank_should_not_error() {
1u128
);

PaymentServices::on_container_author_noted(&1, 1, 1.into());
PaymentServices::on_container_authors_noted(&[AuthorNotingInfo {
author: 1,
block_number: 1,
para_id: 1.into(),
}]);

assert_eq!(
Balances::balance(&crate::Pallet::<Test>::parachain_tank(1.into())),
Expand Down Expand Up @@ -456,7 +469,11 @@ fn tip_should_be_charged_on_collators_assignment() {
false
));

PaymentServices::on_container_author_noted(&1, 1, para_id.into());
PaymentServices::on_container_authors_noted(&[AuthorNotingInfo {
author: 1,
block_number: 1,
para_id: para_id.into(),
}]);

let (assignment_cost, _weight) =
<Test as crate::Config>::ProvideCollatorAssignmentCost::collator_assignment_cost(
Expand Down
Loading

0 comments on commit 104163e

Please sign in to comment.