Skip to content
This repository was archived by the owner on Jan 20, 2026. It is now read-only.

CheckTx for txs from peer proposal#26

Merged
BrandonWeng merged 2 commits intomainfrom
tony-chen-checktx-for-proposed-block
Dec 9, 2022
Merged

CheckTx for txs from peer proposal#26
BrandonWeng merged 2 commits intomainfrom
tony-chen-checktx-for-proposed-block

Conversation

@codchen
Copy link
Collaborator

@codchen codchen commented Nov 22, 2022

Describe your changes and provide context

At the moment, if a tx is all of the following:

  • proposal by tx key is not turned on
  • received as part of a block proposal from other peers
  • not received via gossip on mempool level
    then CheckTx won't be run for it, which could cause problems with ante handlers that are CheckTx-only (e.g. min gas config check).

This PR makes it such that for txs from peer proposal that are not already in the local mempool, the validator will run CheckTx for them before prevoting the proposed block. Note that CheckTx will also insert the tx into the local mempool if the application check passes.

Testing performed to validate your change

tested with local chain
will test with multiple nodes with mempool gossiping turned off once merged and released

Comment on lines +1650 to +1652
if block == nil {
logger.Debug("prevote step: ProposalBlock is nil")
cs.signAddVote(ctx, tmproto.PrevoteType, nil, types.PartSetHeader{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah that's right. Removed

proposalBlock := cs.buildProposalBlock(height, block.Header, block.LastCommit, block.Evidence, block.ProposerAddress, txKeys)
if proposalBlock == nil {
if block == nil {
logger.Debug("prevote step: ProposalBlock is nil")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make this Error or info instead? Unless we expect the proposal block to be nil often

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah this should be Error. Updated

for _, tx := range cs.ProposalBlock.Txs {
blockTxKeyToTx[tx.Key()] = tx
}
for _, missingTxKey := range missingTxKeys {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance of this causing a race condition between different validator nodes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this part is likely to run differently for different validators, which is kind of the point of doing it here, since different validators will have different min gas app configs so some txs might be accepted by some vals but rejected by others, and we want those who would reject those txs to reject (i.e. vote nil) its enclosing block right here. Since this isn't part of block processing, difference in validator behaviors should be fine

@BrandonWeng
Copy link
Contributor

Tested this out e2e with LT cluster on 2.0.0beta, going to merge this

@BrandonWeng BrandonWeng merged commit ba128ae into main Dec 9, 2022
@BrandonWeng BrandonWeng deleted the tony-chen-checktx-for-proposed-block branch December 9, 2022 16:25
BrandonWeng added a commit to sei-protocol/sei-cosmos that referenced this pull request Dec 9, 2022
## Describe your changes and provide context
sei-protocol/sei-tendermint#26 

## Testing performed to validate your change
BrandonWeng pushed a commit that referenced this pull request Dec 27, 2022
* CheckTx for txs from peer proposal

* address comments
Timwood0x10 pushed a commit to Timwood0x10/sei-tendermint that referenced this pull request Jun 7, 2023
[acl] Refactor message dependency mapping to use single key instead module+message
masih pushed a commit to sei-protocol/sei-chain that referenced this pull request Sep 29, 2025
## Describe your changes and provide context
sei-protocol/sei-tendermint#26 

## Testing performed to validate your change
masih pushed a commit to sei-protocol/sei-chain that referenced this pull request Sep 30, 2025
## Describe your changes and provide context
sei-protocol/sei-tendermint#26 

## Testing performed to validate your change
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants