diff --git a/types/tx.go b/types/tx.go index 107f9c4e6..b8c599d8f 100644 --- a/types/tx.go +++ b/types/tx.go @@ -182,13 +182,11 @@ func (t TxRecordSet) Validate(maxSizeBytes int64, otxs Txs) error { // and once by sorting the set of the added, removed, and unmodified transactions indexes, // which, when combined, comprise the complete list of modified transactions. // - // The original list is iterated once and the modified list is iterated multiple times, - // one time alongside each of the 3 indexes for a total of 4 iterations of the modified list. - // Asymptotically, this yields a total runtime of O(N*log(N) + N + 2*M*log(M) + 4*M), - // in the input size of the original list and the input size of the new list respectively. - // A 2 * M performance gain is possible by iterating all of the indexes simultaneously - // alongside the full list, but the multiple iterations were preferred to be more - // readable and maintainable. + // 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 prefered 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. @@ -218,14 +216,14 @@ func (t TxRecordSet) Validate(maxSizeBytes int64, otxs Txs) error { // the caller's data is not altered. otxsCopy := sortedCopy(otxs) - if ix, ok := containsAllTxs(otxsCopy, unmodifiedCopy); !ok { + 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 := containsAllTxs(otxsCopy, removedCopy); !ok { + 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 := containsAnyTxs(otxsCopy, addedCopy); ok { + if ix, ok := containsAny(otxsCopy, addedCopy); ok { return fmt.Errorf("existing transaction incorrectly marked as added, transaction hash: %x", addedCopy[ix].Hash()) } return nil @@ -238,71 +236,42 @@ func sortedCopy(txs Txs) Txs { return cp } -// containsAnyTxs checks that list a contains one of the transactions in list +// 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 containsAnyTxs(a, b []Tx) (int, bool) { - aix, bix := 0, 0 - -nextA: - for ; aix < len(a); aix++ { - for bix < len(b) { - switch bytes.Compare(b[bix], a[aix]) { - case 0: - return bix, true - case -1: - bix++ - // we've reached the end of b, and the last value in b was - // smaller than the value under the iterator of a. - // a's values never get smaller, so we know there are no more matches - // in the list. We can terminate the iteration here. - if bix == len(b) { - return -1, false - } - case 1: - continue nextA - } +func containsAny(a, b []Tx) (int, bool) { + for i, cur := range b { + if _, ok := contains(a, cur); ok { + return i, true } } return -1, false } -// containsAllTxs checks that super contains all of the transactions in the sub +// 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 containsAllTxs(super, sub []Tx) (int, bool) { - // The following iteration assumes sorted lists. - // The checks ensure that all the values in the sorted sub list are present in the sorted super list. - // - // We compare the value under the sub iterator to the value - // under the super iterator. If they match, we advance the - // sub iterator one position. If they don't match, then the value under - // the sub iterator should be greater. - // If it is not, then there is a value in the the sub list that is not present in the - // super list. - subIx := 0 - for _, cur := range super { - if subIx == len(sub) { - return -1, true +func containsAll(super, sub Txs) (int, bool) { + for i, cur := range sub { + if _, ok := contains(super, cur); !ok { + return i, false } - switch bytes.Compare(sub[subIx], cur) { - case 0: - subIx++ - case -1: - return subIx, false - } - } - - // Check that the loop visited all values of the transactions from sub. - // If it did not, then there are values present in these indexes that were not - // present in the super list of transactions. - - if subIx != len(sub) { - return subIx, 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"`