Skip to content

Blobs Fee Adjustment #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Blobs Fee Adjustment #143

wants to merge 1 commit into from

Conversation

gabriele-0201
Copy link
Contributor

This is an extremely experimental implementation, not ready for technical review but rather an example of a possible implementation.

The goal here was to test an approach to fitting TargetedFeeAdjustment to our needs.

What I have done is create a connection between a TargetBlockFullness in bytes and a TargetBlockFullness in ref_time.

The main problem is that there is no easy connection between these two variables. One way I have figured out to deal with this is:

  • Simplify the weight of a submit_blob extrinsic into two parts: base_weight (referring to all the extrinsic actions unrelated to the blob of bytes) and byte_weight (the weight attached to each byte of the blob, which mainly depends on the hash function). The resulting formula for the weight is: base_weight + blob.len() * byte_weight.
  • Each block could contain a different quantity of submitted blobs, and the size of these blobs can easily change over time. However, there is no way (as far as I know) to predict how the ref_time will be composed, whether there will be more extrinsic with little blobs or fewer extrinsic with giant blobs.
    • My solution to this problem is to average this factor at each block and use the result to evaluate the cost of traffic for the next block.
  • Some test logic to increase the TargetBlockSize if blocks were full more than expected.

The final formulas are:

min_ext_len = size in bytes of an empty submit_blob extrinsic

average_blob_size = TotalBlobSize / TotalBlobs

average_submit_blob_weight = weight of a submit_blob extrinsic with a blob the size of average_blob_size

target_ref_time = (TargetBlockSize / (min_ext_len + average_blob_size)) * average_submit_blob_weight

TargetBlockFullness = target_ref_time / max_available_ref_time

Final thoughts:
I may not have considered some factors, and there may be different approaches that could be used rather than sticking solely to TargetedFeeAdjustment.
I am open to suggestions and criticism!

Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.


#[pallet::call_index(1)]
#[pallet::weight(0)]
pub fn update_parameter(
Copy link
Contributor

Choose a reason for hiding this comment

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

This call seems extremely dangerous and broad. What's the purpose of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this call is undoubtedly dangerous. What I would like to achieve is simply a way to allow changes to the parameters of BlobAdjustmentFee with an extrinsic. Before using it, some necessary changes would be assigning proper weights and instead of accepting the key as bytes, it might be better to accept an enum that encapsulates both the key and value to avoid potential incorrect storage insertions.

The current implementation is just an experiment to demonstrate how parameters could be changed by governance/sudo

Copy link
Contributor

Choose a reason for hiding this comment

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

There is already system_setStorage for setting storage arbitrarily - we should use that in favor of introducing another call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uh, that's true! This makes that call completely useless. I will delete it

This was referenced Dec 20, 2023
// Define TargetBlockFullness based on our expectations
// of how we want our blocks to be full
// TODO: refactor
let avarage_blob_size = total_blob_size / total_blobs;
Copy link
Contributor

Choose a reason for hiding this comment

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

divide by zero?

namespace_id: 0.into(),
blob: vec![],
})
.size_hint() as u32;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a constant

@rphmeier
Copy link
Contributor

closing in favor of #149

@rphmeier rphmeier closed this Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants