-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Take into account proof size for transaction payment and priority #13958
Conversation
bot rebase |
…-all-weight-dimensions
Rebased |
bot rebase |
…-all-weight-dimensions
Rebased |
Rebased |
The ref time is limited to 2sec currently while proof size has no limit (u64::Max). The ideal block fullness with normal transactions is set to 25% (0.5 sec, 4.6 * 10^6 terabytes). With current values, practically ref time will always be the limiting dimension and proof size will never be taken into account. Update: Received feedback that we should not charge for proof size on relay chain so the above makes sense. |
bot rebase |
…-all-weight-dimensions
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM codewise
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
I looked at statemine and bifrost and they seem to have pretty empty blocks averaging less than 1% fullness by both The full results are here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM but I think the labeling is wrong here: This PR will significantly affect parachains and should be labled as note-worthy and have direction for parachain teams to consider.
Does each parachain use separate PoV limits? Cumulus picks the PoV limit from here https://github.com/paritytech/polkadot/blob/master/primitives/src/v5/mod.rs#L385. |
Yes I think still different parachains assign different PoV limit via |
…ritytech#13958) * use both proof size and weight * old tests pass, todo: add tests for weight proof size * refactor priority calculation * refactor * normalize dimensions * refactor * update comments * use higher resolution * test multiplier can grow * restore ref time test cases * fix hacky test * fmt * update tests * revert to original error rate * update targetedFeeAdjustment doc * Update frame/transaction-payment/src/lib.rs Co-authored-by: Oliver Tale-Yazdi <[email protected]> * import defensive --------- Co-authored-by: parity-processbot <> Co-authored-by: Oliver Tale-Yazdi <[email protected]>
closes #13898.
Currently,
TargetedFeeAdjustment
takes into account onlyref_time
dimension of the weight. This is not an issue with relay chain since we should not charge transactions forproof_size
on the relay chain. On parachain though, PoV size is limited to 5 MB and an attacker could fill the block with storage size without paying an appropriate fee.These changes will take into account
proof_size
while adjusting the fee. It does so by first taking a ratio ofdimension/max_dimension
and taking the largest ratio. We call itlimiting dimension
as it is the scarcer resource and use this dimension for the calculation of the next fee multiplier.The PR also takes into account
proof_size
for calculation of transaction priority. We look into how many transactions we can fill in the block by looking at the resource (dimension) that is depleted more (hence called limiting dimension) while filling the block.Considerations for parachains using pallet
transaction-payment
Care needs to be taken to set the max PoV size for a chain. A really high value would mean block is never full based on the
proof_size
dimension, while a too low value would mean block is constantly full and transaction fees would keep increasing for the chain. An optimal maximum PoV size would ensure a high correlation between the block fullness based on each weight dimension.