celestiaorg / celestia-core

Celestia node software based on Tendermint.
https://celestia.org/
Apache License 2.0
470 stars 244 forks source link

RequestPrepareProposal has unclear comments #1390

Closed rootulp closed 1 week ago

rootulp commented 2 weeks ago

Context

See Slack thread

https://github.com/celestiaorg/celestia-core/blob/fb9d6562e5ed3bbd7362aa5530920b09c28cb721/proto/tendermint/abci/types.proto#L129-L135

Problem

I think these comments aren't clear.

// block_data is an array of transactions that will be included in a block, sent to the app for possible modifications.

block_data is an array of candidate transactions that may be included in a block. The array is sent to the application to filter out any invalid transactions and re-order all SDK transactions before blob transactions. Note the transactions themselves aren't modified.

// applications can not exceed the size of the data passed to it.

IDK what this means because the application does in fact return a BlockData with a size greater than the data passed in. The BlockData that the application returns includes a SquareSize and hash (data root).

// If an application decides to populate block_data with extra information, they can not exceed this value.

IMO this should clarify that the total BlockData can't exceed BlockDataSize. The way it's written currently made me think that just the extra information can no exceed BlockDataSize.

Proposal

Update the comments to make them clearer.

Note: the CometBFT RequestPrepareProposal type looks a bit different from ours so we can't copy their comments.

rootulp commented 1 week ago

Closed by https://github.com/celestiaorg/celestia-core/pull/1393