From e0663e1837a2b215e9c5477b94ccc3b3a4b1b36b Mon Sep 17 00:00:00 2001 From: William Banfield Date: Wed, 17 Aug 2022 11:48:44 -0400 Subject: [PATCH] code builds --- abci/example/kvstore/persistent_kvstore.go | 23 +-- abci/types/application.go | 9 +- state/execution.go | 12 +- test/e2e/app/app.go | 10 +- types/tx.go | 196 ++------------------- 5 files changed, 33 insertions(+), 217 deletions(-) diff --git a/abci/example/kvstore/persistent_kvstore.go b/abci/example/kvstore/persistent_kvstore.go index d8c932201..61f3a53fe 100644 --- a/abci/example/kvstore/persistent_kvstore.go +++ b/abci/example/kvstore/persistent_kvstore.go @@ -172,7 +172,7 @@ func (app *PersistentKVStoreApplication) ApplySnapshotChunk( func (app *PersistentKVStoreApplication) PrepareProposal( req types.RequestPrepareProposal) types.ResponsePrepareProposal { - return types.ResponsePrepareProposal{TxRecords: app.substPrepareTx(req.Txs, req.MaxTxBytes)} + return types.ResponsePrepareProposal{Txs: app.substPrepareTx(req.Txs, req.MaxTxBytes)} } func (app *PersistentKVStoreApplication) ProcessProposal( @@ -317,32 +317,19 @@ func (app *PersistentKVStoreApplication) execPrepareTx(tx []byte) types.Response // substPrepareTx substitutes all the transactions prefixed with 'prepare' in the // proposal for transactions with the prefix stripped. -// It marks all of the original transactions as 'REMOVED' so that -// Tendermint will remove them from its mempool. -func (app *PersistentKVStoreApplication) substPrepareTx(blockData [][]byte, maxTxBytes int64) []*types.TxRecord { - trs := make([]*types.TxRecord, 0, len(blockData)) - var removed []*types.TxRecord +func (app *PersistentKVStoreApplication) substPrepareTx(blockData [][]byte, maxTxBytes int64) [][]byte { + txs := make([][]byte, 0, len(blockData)) var totalBytes int64 for _, tx := range blockData { txMod := tx - action := types.TxRecord_UNMODIFIED if isPrepareTx(tx) { - removed = append(removed, &types.TxRecord{ - Tx: tx, - Action: types.TxRecord_REMOVED, - }) txMod = bytes.TrimPrefix(tx, []byte(PreparePrefix)) - action = types.TxRecord_ADDED } totalBytes += int64(len(txMod)) if totalBytes > maxTxBytes { break } - trs = append(trs, &types.TxRecord{ - Tx: txMod, - Action: action, - }) + txs = append(txs, tx) } - - return append(trs, removed...) + return txs } diff --git a/abci/types/application.go b/abci/types/application.go index 2bc107091..0913ea463 100644 --- a/abci/types/application.go +++ b/abci/types/application.go @@ -95,19 +95,16 @@ func (BaseApplication) ApplySnapshotChunk(req RequestApplySnapshotChunk) Respons } func (BaseApplication) PrepareProposal(req RequestPrepareProposal) ResponsePrepareProposal { - trs := make([]*TxRecord, 0, len(req.Txs)) + txs := make([][]byte, 0, len(req.Txs)) var totalBytes int64 for _, tx := range req.Txs { totalBytes += int64(len(tx)) if totalBytes > req.MaxTxBytes { break } - trs = append(trs, &TxRecord{ - Action: TxRecord_UNMODIFIED, - Tx: tx, - }) + txs = append(txs, tx) } - return ResponsePrepareProposal{TxRecords: trs} + return ResponsePrepareProposal{Txs: txs} } func (BaseApplication) ProcessProposal(req RequestProcessProposal) ResponseProcessProposal { diff --git a/state/execution.go b/state/execution.go index 0cd53e5bb..b8e328d84 100644 --- a/state/execution.go +++ b/state/execution.go @@ -136,19 +136,13 @@ func (blockExec *BlockExecutor) CreateProposalBlock( // error for now (the production code calling this function is expected to panic). return nil, err } - txrSet := types.NewTxRecordSet(rpp.TxRecords) - if err := txrSet.Validate(maxDataBytes, block.Txs); err != nil { + txl := types.ToTxs(rpp.Txs) + if err := txl.Validate(maxDataBytes); err != nil { return nil, err } - for _, rtx := range txrSet.RemovedTxs() { - if err := blockExec.mempool.RemoveTxByKey(rtx.Key()); err != nil { - blockExec.logger.Debug("error removing transaction from the mempool", "error", err, "tx hash", rtx.Hash()) - } - } - itxs := txrSet.IncludedTxs() - return state.MakeBlock(height, itxs, commit, evidence, proposerAddr), nil + return state.MakeBlock(height, txl, commit, evidence, proposerAddr), nil } func (blockExec *BlockExecutor) ProcessProposal( diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go index ad34fd7e4..b48242ffc 100644 --- a/test/e2e/app/app.go +++ b/test/e2e/app/app.go @@ -259,20 +259,16 @@ func (app *Application) ApplySnapshotChunk(req abci.RequestApplySnapshotChunk) a func (app *Application) PrepareProposal( req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - // None of the transactions are modified by this application. - trs := make([]*abci.TxRecord, 0, len(req.Txs)) + txs := make([][]byte, 0, len(req.Txs)) var totalBytes int64 for _, tx := range req.Txs { totalBytes += int64(len(tx)) if totalBytes > req.MaxTxBytes { break } - trs = append(trs, &abci.TxRecord{ - Action: abci.TxRecord_UNMODIFIED, - Tx: tx, - }) + txs = append(txs, tx) } - return abci.ResponsePrepareProposal{TxRecords: trs} + return abci.ResponsePrepareProposal{Txs: txs} } // ProcessProposal implements part of the Application interface. diff --git a/types/tx.go b/types/tx.go index ade9c94e4..8ed067ff7 100644 --- a/types/tx.go +++ b/types/tx.go @@ -5,9 +5,7 @@ import ( "crypto/sha256" "errors" "fmt" - "sort" - abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/crypto/tmhash" tmbytes "github.com/tendermint/tendermint/libs/bytes" @@ -98,6 +96,25 @@ func (txs Txs) Less(i, j int) bool { return bytes.Compare(txs[i], txs[j]) == -1 } +func ToTxs(txl [][]byte) Txs { + txs := make([]Tx, 0, len(txl)) + for _, tx := range txl { + txs = append(txs, tx) + } + return txs +} + +func (txs Txs) Validate(maxSizeBytes int64) error { + var size int64 + for _, tx := range txs { + size += int64(len(tx)) + if size > maxSizeBytes { + return fmt.Errorf("transaction data size exceeds maximum %d", maxSizeBytes) + } + } + return nil +} + // ToSliceOfBytes converts a Txs to slice of byte slices. func (txs Txs) ToSliceOfBytes() [][]byte { txBzs := make([][]byte, len(txs)) @@ -107,181 +124,6 @@ func (txs Txs) ToSliceOfBytes() [][]byte { return txBzs } -// TxRecordSet contains indexes into an underlying set of transactions. -// These indexes are useful for validating and working with a list of TxRecords -// from the PrepareProposal response. -// -// Only one copy of the original data is referenced by all of the indexes but a -// transaction may appear in multiple indexes. -type TxRecordSet struct { - // all holds the complete list of all transactions from the original list of - // TxRecords. - all Txs - - // included is an index of the transactions that will be included in the block - // and is constructed from the list of both added and unmodified transactions. - // included maintains the original order that the transactions were present - // in the list of TxRecords. - included Txs - - // added, unmodified, removed, and unknown are indexes for each of the actions - // that may be supplied with a transaction. - // - // Because each transaction only has one action, it can be referenced by - // at most 3 indexes in this data structure: the action-specific index, the - // included index, and the all index. - added Txs - unmodified Txs - removed Txs - unknown Txs -} - -// NewTxRecordSet constructs a new set from the given transaction records. -// The contents of the input transactions are shared by the set, and must not -// be modified during the lifetime of the set. -func NewTxRecordSet(trs []*abci.TxRecord) TxRecordSet { - txrSet := TxRecordSet{ - all: make([]Tx, len(trs)), - } - for i, tr := range trs { - - txrSet.all[i] = Tx(tr.Tx) - - // The following set of assignments do not allocate new []byte, they create - // pointers to the already allocated slice. - switch tr.GetAction() { - case abci.TxRecord_UNKNOWN: - txrSet.unknown = append(txrSet.unknown, txrSet.all[i]) - case abci.TxRecord_UNMODIFIED: - txrSet.unmodified = append(txrSet.unmodified, txrSet.all[i]) - txrSet.included = append(txrSet.included, txrSet.all[i]) - case abci.TxRecord_ADDED: - txrSet.added = append(txrSet.added, txrSet.all[i]) - txrSet.included = append(txrSet.included, txrSet.all[i]) - case abci.TxRecord_REMOVED: - txrSet.removed = append(txrSet.removed, txrSet.all[i]) - } - } - return txrSet -} - -// IncludedTxs returns the transactions marked for inclusion in a block. This -// list maintains the order that the transactions were included in the list of -// TxRecords that were used to construct the TxRecordSet. -func (t TxRecordSet) IncludedTxs() []Tx { - return t.included -} - -// RemovedTxs returns the transactions marked for removal by the application. -func (t TxRecordSet) RemovedTxs() []Tx { - return t.removed -} - -// Validate checks that the record set was correctly constructed from the original -// list of transactions. -func (t TxRecordSet) Validate(maxSizeBytes int64, otxs Txs) error { - if len(t.unknown) > 0 { - return fmt.Errorf("%d transactions marked unknown (first unknown hash: %x)", len(t.unknown), t.unknown[0].Hash()) - } - - // The following validation logic performs a set of sorts on the data in the TxRecordSet indexes. - // It sorts the original transaction list, otxs, once. - // It sorts the new transaction list twice: once when sorting 'all', the total list, - // and once by sorting the set of the added, removed, and unmodified transactions indexes, - // which, when combined, comprise the complete list of modified transactions. - // - // Each of the added, removed, and unmodified indices is then iterated and once - // and each value index is checked against the sorted original list for containment. - // Asymptotically, this yields a total runtime of O(N*log(N) + 2*M*log(M) + M*log(N)). - // in the input size of the original list, N, and the input size of the new list, M, respectively. - // Performance gains are likely possible, but this was preferred for readability and maintainability. - - // Sort a copy of the complete transaction slice so we can check for - // duplication. The copy is so we do not change the original ordering. - // Only the slices are copied, the transaction contents are shared. - allCopy := sortedCopy(t.all) - - for i, cur := range allCopy { - // allCopy is sorted, so any duplicated data will be adjacent. - if i+1 < len(allCopy) && bytes.Equal(cur, allCopy[i+1]) { - return fmt.Errorf("found duplicate transaction with hash: %x", cur.Hash()) - } - } - - // create copies of each of the action-specific indexes so that order of the original - // indexes can be preserved. - addedCopy := sortedCopy(t.added) - removedCopy := sortedCopy(t.removed) - unmodifiedCopy := sortedCopy(t.unmodified) - - var size int64 - for _, cur := range append(unmodifiedCopy, addedCopy...) { - size += int64(len(cur)) - if size > maxSizeBytes { - return fmt.Errorf("transaction data size exceeds maximum %d", maxSizeBytes) - } - } - - // make a defensive copy of otxs so that the order of - // the caller's data is not altered. - otxsCopy := sortedCopy(otxs) - - if ix, ok := containsAll(otxsCopy, unmodifiedCopy); !ok { - return fmt.Errorf("new transaction incorrectly marked as removed, transaction hash: %x", unmodifiedCopy[ix].Hash()) - } - - if ix, ok := containsAll(otxsCopy, removedCopy); !ok { - return fmt.Errorf("new transaction incorrectly marked as removed, transaction hash: %x", removedCopy[ix].Hash()) - } - if ix, ok := containsAny(otxsCopy, addedCopy); ok { - return fmt.Errorf("existing transaction incorrectly marked as added, transaction hash: %x", addedCopy[ix].Hash()) - } - return nil -} - -func sortedCopy(txs Txs) Txs { - cp := make(Txs, len(txs)) - copy(cp, txs) - sort.Sort(cp) - return cp -} - -// containsAny checks that list a contains one of the transactions in list -// b. If a match is found, the index in b of the matching transaction is returned. -// Both lists must be sorted. -func containsAny(a, b []Tx) (int, bool) { - for i, cur := range b { - if _, ok := contains(a, cur); ok { - return i, true - } - } - return -1, false -} - -// containsAll checks that super contains all of the transactions in the sub -// list. If not all values in sub are present in super, the index in sub of the -// first Tx absent from super is returned. -func containsAll(super, sub Txs) (int, bool) { - for i, cur := range sub { - if _, ok := contains(super, cur); !ok { - return i, false - } - } - return -1, true -} - -// contains checks that the sorted list, set contains elem. If set does contain elem, then the -// index in set of elem is returned. -func contains(set []Tx, elem Tx) (int, bool) { - n := sort.Search(len(set), func(i int) bool { - return bytes.Compare(elem, set[i]) <= 0 - }) - if n == len(set) || !bytes.Equal(elem, set[n]) { - return -1, false - } - return n, true -} - // TxProof represents a Merkle proof of the presence of a transaction in the Merkle tree. type TxProof struct { RootHash tmbytes.HexBytes `json:"root_hash"`