abci++: update PrepareProposal API to accept just a list of bytes (#9283)

This changes the ResponsePrepareProposal type, substituting the []TxRecord for just []bytes. This change is made in the .proto file and in all necessary additional places in the code.
This commit is contained in:
William Banfield
2022-08-18 16:33:19 -04:00
committed by GitHub
parent fb9afcc969
commit 1a4d9397e9
16 changed files with 314 additions and 1062 deletions

View File

@@ -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"`

View File

@@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
tmrand "github.com/tendermint/tendermint/libs/rand"
ctest "github.com/tendermint/tendermint/libs/test"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
@@ -48,179 +47,6 @@ func TestTxIndexByHash(t *testing.T) {
}
}
func TestValidateTxRecordSet(t *testing.T) {
t.Run("should error on total transaction size exceeding max data size", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{6, 7, 8, 9, 10}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(9, []Tx{})
require.Error(t, err)
})
t.Run("should not error on removed transaction size exceeding max data size", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{6, 7, 8, 9}),
},
{
Action: abci.TxRecord_REMOVED,
Tx: Tx([]byte{10}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(9, []Tx{[]byte{10}})
require.NoError(t, err)
})
t.Run("should error on duplicate transactions with the same action", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{100}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{200}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{})
require.Error(t, err)
})
t.Run("should error on duplicate transactions with mixed actions", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{100}),
},
{
Action: abci.TxRecord_REMOVED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{200}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{})
require.Error(t, err)
})
t.Run("should error on new transactions marked UNMODIFIED", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_UNMODIFIED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{})
require.Error(t, err)
})
t.Run("should error on new transactions marked REMOVED", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_REMOVED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{})
require.Error(t, err)
})
t.Run("should error on existing transaction marked as ADDED", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{5, 4, 3, 2, 1}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{6}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{{0}, {1, 2, 3, 4, 5}})
require.Error(t, err)
})
t.Run("should error if any transaction marked as UNKNOWN", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_UNKNOWN,
Tx: Tx([]byte{1, 2, 3, 4, 5}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{})
require.Error(t, err)
})
t.Run("TxRecordSet preserves order", func(t *testing.T) {
trs := []*abci.TxRecord{
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{100}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{99}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{55}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{12}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{66}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{9}),
},
{
Action: abci.TxRecord_ADDED,
Tx: Tx([]byte{17}),
},
}
txrSet := NewTxRecordSet(trs)
err := txrSet.Validate(100, []Tx{})
require.NoError(t, err)
for i, tx := range txrSet.IncludedTxs() {
require.Equal(t, Tx(trs[i].Tx), tx)
}
})
}
func TestValidTxProof(t *testing.T) {
cases := []struct {
txs Txs