feat: add time to RequestPreparePropsosal
#1081
Conversation
proto/tendermint/abci/types.proto
Outdated
| // height is the height of the proposal block | ||
| int64 height = 4; | ||
| // time is the time that will end up in the header. This is the voting power | ||
| // weighted median of the last commit. | ||
| google.protobuf.Timestamp time = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true]; | ||
| // version is the block and app versions for the proposal. | ||
| tendermint.version.Consensus version = 6 [(gogoproto.nullable) = false]; |
There was a problem hiding this comment.
per op, the height and the application version aren't really needed since the application keeps track of those. However the block version is not kept track of afaik, so went ahead and added it as well. Happy to remove if it is desired to keep separate or requires further discussion
There was a problem hiding this comment.
version isn't also part of cometbft v0.37.x so I don't think it's necessary to add it here if we're not using it.
Ref of our main branch:
There was a problem hiding this comment.
yeah I noticed that, I figured there could be an scenario where we use the block version for single binary syncs/upgrades, but we can add it then if needed so removed in fde59be
proto/tendermint/abci/types.proto
Outdated
| // height is the height of the proposal block | ||
| int64 height = 4; | ||
| // time is the time that will end up in the header. This is the voting power | ||
| // weighted median of the last commit. | ||
| google.protobuf.Timestamp time = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true]; | ||
| // version is the block and app versions for the proposal. | ||
| tendermint.version.Consensus version = 6 [(gogoproto.nullable) = false]; |
There was a problem hiding this comment.
version isn't also part of cometbft v0.37.x so I don't think it's necessary to add it here if we're not using it.
Ref of our main branch:
| int64 height = 4; | ||
| // time is the time that will end up in the header. This is the voting power | ||
| // weighted median of the last commit. | ||
| google.protobuf.Timestamp time = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true]; | ||
| } |
There was a problem hiding this comment.
(Note for myself mainly): Compare with:
- comet main: https://github.com/cometbft/cometbft/blob/1c6f86c840c64f83f4409ccca6b664234866cc38/proto/tendermint/abci/types.proto#L129-L142
- v0.36.x: https://github.com/cometbft/cometbft/blob/6321ef2936fbbaf0e3df575982c9485f832a8543/proto/tendermint/abci/types.proto#L105-L118
- v0.37.x: https://github.com/cometbft/cometbft/blob/be0df2e241f0bd20d51ea2b63e25583ea73dbe21/proto/tendermint/abci/types.proto#L124-L137
| var timestamp time.Time | ||
| if height == state.InitialHeight { | ||
| timestamp = state.LastBlockTime // genesis time | ||
| } else { | ||
| timestamp = MedianTime(commit, state.LastValidators) | ||
| } |
There was a problem hiding this comment.
Description
Adds the last commit median time to
RequestPreparePropsosal. This allows the application to accurately calculate the spendable funds of vesting accounts, since this is the time used when executing the transactions and when evaluating the propsoal duringProcessProposal.The height was also added, but it doesn't have to be since the application can calculate that value. That value is also passed in other versions of comet, and we'll have to add it we want to unfork from the sdk so I figured why not. Happy to remove it as well.