-
Notifications
You must be signed in to change notification settings - Fork 252
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
feat: use kvstore to fetch gas per blob byte #1095
feat: use kvstore to fetch gas per blob byte #1095
Conversation
nice! what's left for this? is this blocked by anything? |
9deaa45
to
cf5cd4e
Compare
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.
great work! thanks for adding the really good tests!!
}, | ||
{ | ||
name: "1024 byte blob", // occupies 3 shares because share prefix (e.g. namespace, info byte) | ||
msg: types.MsgPayForBlob{BlobSize: 1024}, | ||
wantGasConsumed: uint64(12288), // 3 shares * 512 bytes per share * 8 gas per byte = 12288 gas | ||
wantGasConsumed: uint64(13348), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 12288 gas |
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.
the sum wasn't updated in the comment
wantGasConsumed: uint64(13348), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas
// calculate gas per message share by fetching the constant share size and the gas cost per byte from the KV store | ||
gasPerMsgShare := appconsts.ShareSize * k.GasPerBlobByte(ctx) |
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.
[proposal]
message => blob
Replaces gas per blob byte constant with stored param
-- Note - To be merged after #1031