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

Add performABCIHandler to testing pkg to replace internal logic of SendMsgs/NextBlock #3997

Open
3 tasks
colin-axner opened this issue Jul 3, 2023 · 0 comments
Labels
testing Testing package and unit/integration tests type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.

Comments

@colin-axner
Copy link
Contributor

Summary

The testing pkg currently has two main entry points to commit a block:

  • chain.SendMsgs (receives a list of msgs)
  • chain.NextBlock (essentially receives empty list of msgs)

These two functions should call the same underlying handler which performs the expected steps an ABCI driver would call.

Problem Definition

Currently the testing pkg improperly mocks the flow of an ABCI application. This can lead to issues like improperly set validator set hashes, #1668. We should follow the flow of the ABCI handler to increase our confidence in the correctness of the testing pkg.

See my comment on how the testing pkg currently flows.

The way it should flow is as follows:

// block proposer proposes a block

// prevotes/precommit stages 

// a header is committed to by 2/3+ validator signing over the proposed header

// finalize block occurs (begin, deliver, end)

// commit() (app hash created)

Proposal

We should add a function like:

// performABCIHandler performs a block execution in the same manner that an ABCI driver will call 
// an ABCI application to do so. The format is as follows:
//
// - commit the currently proposed block header (signing with 2/3+ validator set)
// - executing application logic via a call to FinalizeBlock
// - commit the current application state to the underlying storage tree (IAVL)
// - increment the block by setting the latest committed header and generating a new uncommitted header
func (chain *TestChain) performABCIHandler(msgs []sdk.Msg) (*abcitypes.ExecTxResult, error) {
    signedHeader, err := CommitHeader(chain.CurrentUncommittedHeader, chain.Vals, chain.Signers)
    require.NoError(chain.TB, err)
    
    res, err := chain.executeBlockWithMsgs(msgs)
    if err != nil {
        return nil, err
    }
    
    chain.App.Commit()
    
    chain.LatestCommittedHeader = committedHeader
        
    // val set changes returned from previous block get applied to the next validators
    // of this block. See tendermint spec for details.
    chain.Vals = chain.NextVals
    chain.NextVals = ApplyValSetChanges(chain, chain.Vals, res.ValidatorUpdates)

    chain.CurrentUncommittedHeader = chain.proposeNextBlock(signedHeader)
    
    // parse out abcitypes.ExecTxResult
    return res, nil
}

func (chain *TestChain) executeBlockWithMsgs(msgs []sdk.Msg) (*abcitypes.FinalizeBlockResponse error) {
    tx, err := simtestutil.GenSignedMockTx(...)
    require.NoError(tb, err)
    
    txBytes, err := txCfg.TxEncoder()(tx)
    require.NoError(tb, err)

    return chain.App.FinalizeBlock(&abci.RequestFinalizeBlock{
		Height:             chain.CurrentHeader.Height,
		Time:               chain.CurrentHeader.GetTime(),
		NextValidatorsHash: chain.NextVals.Hash(),
		Txs:                [][]byte{txBytes},
    })
}

func (chain *TestChain) proposeNextBlock() cmtproto.Header {
    return chain.CurrentHeader = cmtproto.Header{
		ChainID: chain.ChainID,
		Height:  chain.App.LastBlockHeight() + 1,
		AppHash: chain.App.LastCommitID().Hash,
		// NOTE: the time is increased by the coordinator to maintain time synchrony amongst
		// chains.
		Time:               chain.CurrentHeader.Time,
		ValidatorsHash:     chain.Vals.Hash(),
		NextValidatorsHash: chain.NextVals.Hash(),
		ProposerAddress:    chain.CurrentHeader.ProposerAddress,
	}
 }

Send msgs and next block can then reuse this function:

func (chain *TestChain) SendMsgs(msgs ...sdk.Msg) (*abci.ExecTxResult, error) {
	if chain.SendMsgsOverride != nil {
		return chain.SendMsgsOverride(msgs...)
	}
        
        res, err := chain.performABCIHandler(msgs)
        if err != nil {
            return nil, err
        }
        
        // increment sequence for successful transaction execution
	err = chain.SenderAccount.SetSequence(chain.SenderAccount.GetSequence() + 1)
	if err != nil {
		return nil, err
	}
	
	return res, nil
}

func (chain *TestChain) NextBlock() {
    res, err := chain.perfomABCIHandler(nil)
    require.NoError(chain.TB, err)
    require.NotNil(chain.TB, res)
}

As a side note, we may be eventually able to delete:

  • coordinator.CommitBlock
  • coordinator.CommitNBlocks

but the handle time increments so they cannot be replaced until we remove manual handling of time

ref:


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Testing package and unit/integration tests type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
Status: Todo 🏃
Development

No branches or pull requests

2 participants