Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Update Weights for Claims Pallet #1036

Merged
merged 1 commit into from
Apr 27, 2020
Merged

Update Weights for Claims Pallet #1036

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
18 changes: 16 additions & 2 deletions runtime/common/src/claims.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,13 @@ decl_module! {
/// - One deposit event.
///
/// Total Complexity: O(1)
/// ----------------------------
/// Base Weight: 622.6 µs
/// DB Weight:
/// - Read: Claims, Total, Claims Vesting, Vesting Vesting, Balance Lock, Account
/// - Write: Vesting Vesting, Account, Balance Lock, Total, Claim, Claims Vesting
/// </weight>
#[weight = 1_000_000_000]
#[weight = T::DbWeight::get().reads_writes(6, 6) + 650_000_000]
fn claim(origin, dest: T::AccountId, ethereum_signature: EcdsaSignature) {
ensure_none(origin)?;

Expand Down Expand Up @@ -229,8 +234,13 @@ decl_module! {
/// - Up to one storage write to add a new vesting schedule.
///
/// Total Complexity: O(1)
/// ---------------------
/// Base Weight: 25.64 µs
/// DB Weight:
/// - Reads: Total
/// - Writes: Total, Claims, Vesting
/// </weight>
#[weight = 30_000_000]
#[weight = T::DbWeight::get().reads_writes(1, 3) + 25_000_000]
fn mint_claim(origin,
who: EthereumAddress,
value: BalanceOf<T>,
Expand Down Expand Up @@ -292,6 +302,10 @@ impl<T: Trait> sp_runtime::traits::ValidateUnsigned for Module<T> {
const PRIORITY: u64 = 100;

match call {
// <weight>
// Base Weight: 370 µs
// DB Weight: 1 Read (Claims)
// </weight>
Copy link
Contributor

@gui1117 gui1117 Apr 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

So validate_unsigned time computation is not part of claim weight because it less than ExtrinsicBaseWeight ?

EDIT: currently ExtrinsicBaseWeight seems a misleading because actually it is the weight for importing a signed extrinsic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and we noted that in our Riot chat. I think this is supposed to be "good enough for now".

I think though, to be safe, I should include this weight into the main extrinsic until we polish all the details.

Copy link
Contributor

@gui1117 gui1117 Apr 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

yes, IMHO currently ExtrinsicBaseWeight seems to be 10µs so I would say better to put into the extrinsic as well. In the future maybe we should have SignedExtrinsicBaseWeight UnsignedExtrinsicBaseWeight.

(Also the benchmark could include both so read/writes are not duplicated (but here its fine))

Copy link
Member Author

@shawntabrizi shawntabrizi Apr 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

In the future maybe we should have SignedExtrinsicBaseWeight UnsignedExtrinsicBaseWeight.

yes, I think that will be the case, but lots of refactoring to do in general around weights for the long term.

Call::claim(account, ethereum_signature) => {
let data = account.using_encoded(to_ascii_hex);
let maybe_signer = Self::eth_recover(&ethereum_signature, &data);
Expand Down