Skip to content
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

Charge gas per message share #431

Closed
Tracked by #514 ...
evan-forbes opened this issue May 21, 2022 · 2 comments · Fixed by #946
Closed
Tracked by #514 ...

Charge gas per message share #431

evan-forbes opened this issue May 21, 2022 · 2 comments · Fixed by #946
Assignees
Labels
specs directly relevant to the specs

Comments

@evan-forbes
Copy link
Member

evan-forbes commented May 21, 2022

Padding was removed from messages entirely in #419 as it was causing complexity. We can however still pay for padding by increasing the declared size of the message in the MsgPayForData and MsgWirePayForData. While this will slightly complicate ValidateBasic, it is definitely possible.

It's worth noting that paying for padding isn't enforceable. As previously discussed conversations around fee burning, due to tendermint's deferred execution model, enforcing that a message declare the correct padded size is not possible atm. It would be trivial for a block producer to include a MsgPayForData transaction that declares the size to be 0 or whatever size they want. This isn't really an issue, as its the same as a block producer including a message with a zero fee. However, adding a check in ValidateBasic would stop messages that are not paying for padding from being propagated by nodes running unmodified software.

@evan-forbes
Copy link
Member Author

to update from a sync: we still want to include the in message padding when calculating gas costs. This effectively means also including the padding in the last share when calculating gas costs. This means that we will be charging per share used, instead of per byte.

We will not be including padding from the non-interactive defaults into gas costs.

@rootulp rootulp changed the title Pay for padding by default. Pay for last share's padding by default Nov 1, 2022
@adlerjohn adlerjohn added the specs directly relevant to the specs label Nov 1, 2022
@rootulp
Copy link
Collaborator

rootulp commented Nov 2, 2022

we will be charging per share used, instead of per byte

Clarification: I infer this as paying for each byte used by the message shares associated with a PFD (including namespace IDs, info bytes, message length delimiter, and padding). I also infer this as not charging for the amount of transaction shares associated with the PFD.

@rootulp rootulp changed the title Pay for last share's padding by default Charge gas per message share Nov 2, 2022
rootulp added a commit that referenced this issue Nov 8, 2022
Closes #431

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
@rootulp rootulp self-assigned this Nov 11, 2022
rach-id added a commit to rach-id/celestia-app that referenced this issue Nov 16, 2022
Closes celestiaorg#431

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
rach-id added a commit to rach-id/celestia-app that referenced this issue Nov 16, 2022
Closes celestiaorg#431

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
0xchainlover pushed a commit to celestia-org/celestia-app that referenced this issue Aug 1, 2024
Closes celestiaorg/celestia-app#431

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
specs directly relevant to the specs
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants