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

Companion for Weight V2 #5435

Closed
wants to merge 2 commits into from
Closed

Companion for Weight V2 #5435

wants to merge 2 commits into from

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented May 2, 2022

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label May 2, 2022
}

/// Receive messages proof from bridged chain.
///
/// The weight of the call assumes that the transaction always brings outbound lane
/// state update. Because of that, the submitter (relayer) has no benefit of not including
/// this data in the transaction, so reward confirmations lags should be minimal.
#[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight))]
#[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, dispatch_weight.computation()))]
Copy link
Member

Choose a reason for hiding this comment

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

this is surely wrong. the dispatch will also have a PoV component which should be added in, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is handled by the receive_messages_proof_weight. dispatch_weight argument means is a pure dispatch weight - e.g. for call it needs to be call.get_dispatch_info().weight.

Copy link
Member

Choose a reason for hiding this comment

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

i see. is there some reason they're not being handled together?

Copy link
Contributor

Choose a reason for hiding this comment

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

Dispatch weight is declared by the message sender when message is sent from the source (here: bridged) chain. Relay computes this argument by summing weights of all delivered messages. Why: originally we've been delivering encoded calls with prepaid (at the source chain) dispatch. Since we can't call call.get_dispatch_info().weight at the bridged chain (because it don't know the call type + it don't know weights of the other chain), it was provided by the message sender.

Other weight components (like cost of proof verification and others) depend on this chain (here: target) and are computed using regular auto benchmarks.

There's an open issue, related to this dispatch_weight argument and ongoing XCM integration. But overall, scheme will stay the same (separate argument for pure dispatch weight and auto-benchmarks for everything else).

@@ -387,7 +390,7 @@ pub mod pallet {
let declared_weight = T::WeightInfo::receive_messages_proof_weight(
&proof,
messages_count,
dispatch_weight,
dispatch_weight.computation(),
Copy link
Member

Choose a reason for hiding this comment

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

same

0
},
);
actual_weight =
Copy link
Member

Choose a reason for hiding this comment

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

also here. we seem to be disregarding PoV weight.

@@ -495,7 +499,10 @@ pub mod pallet {
declared_weight,
);

Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes })
Ok(PostDispatchInfo {
actual_weight: Some(Weight::from_computation(actual_weight)),
Copy link
Member

Choose a reason for hiding this comment

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

also here?

ump_max_individual_weight: 20 * WEIGHT_PER_MILLIS,
ump_max_individual_weight: Weight::new()
.set_computation(20 * WEIGHT_PER_MILLIS)
.set_bandwidth(5 * 1024 * 1024),
Copy link
Member

Choose a reason for hiding this comment

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

20 ms is ~1% of the 2s expected maximum. 50KB should be about the same proportion of the maximum PoV size.

Suggested change
.set_bandwidth(5 * 1024 * 1024),
.set_bandwidth(50 * 1024),

@@ -204,7 +204,7 @@ impl<T: Config> Pallet<T> {
*q = q.split_off(processed_downward_messages);
}
});
T::DbWeight::get().reads_writes(1, 1)
Weight::from_computation(T::DbWeight::get().reads_writes(1, 1))
Copy link
Member

Choose a reason for hiding this comment

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

One for the parachains team.

@pepyakin pepyakin self-requested a review June 21, 2022 21:09
@shawntabrizi shawntabrizi deleted the shawntabrizi-weight-v2 branch October 10, 2022 14:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants