diff --git a/txcache/README.md b/txcache/README.md index 6330b033..ec9fb7ba 100644 --- a/txcache/README.md +++ b/txcache/README.md @@ -93,7 +93,7 @@ That is, for contract calls, the PPU is not equal to the gas price, but much low Transaction **A** is considered **more valuable (for the Network)** than transaction **B** if **it has a higher PPU**. -If two transactions have the same PPU, they are ordered using an arbitrary, but deterministic rule: the transaction with the higher [fnv32(transactionHash)](https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function) "wins" the comparison. +If two transactions have the same PPU, they are ordered using an arbitrary, but deterministic rule: the transaction with the "lower" transaction hash "wins" the comparison. Pseudo-code: @@ -103,7 +103,7 @@ func isTransactionMoreValuableForNetwork(A, B): return true if A.ppu < B.ppu: return false - return fnv32(A.hash) > fnv32(B.hash) + return A.hash < B.hash ``` ### Paragraph 4 diff --git a/txcache/selection_test.go b/txcache/selection_test.go index 5d358e26..fb8f45c4 100644 --- a/txcache/selection_test.go +++ b/txcache/selection_test.go @@ -15,28 +15,13 @@ func TestTxCache_SelectTransactions_Dummy(t *testing.T) { cache := newUnconstrainedCacheToTest() cache.AddTx(createTx([]byte("hash-alice-4"), "alice", 4)) - require.Equal(t, 3193030061, int(fnv32("hash-alice-4"))) - cache.AddTx(createTx([]byte("hash-alice-3"), "alice", 3)) - require.Equal(t, 3193030058, int(fnv32("hash-alice-3"))) - cache.AddTx(createTx([]byte("hash-alice-2"), "alice", 2)) - require.Equal(t, 3193030059, int(fnv32("hash-alice-2"))) - cache.AddTx(createTx([]byte("hash-alice-1"), "alice", 1)) - require.Equal(t, 3193030056, int(fnv32("hash-alice-1"))) - cache.AddTx(createTx([]byte("hash-bob-7"), "bob", 7)) - require.Equal(t, 187766579, int(fnv32("hash-bob-7"))) - cache.AddTx(createTx([]byte("hash-bob-6"), "bob", 6)) - require.Equal(t, 187766578, int(fnv32("hash-bob-6"))) - cache.AddTx(createTx([]byte("hash-bob-5"), "bob", 5)) - require.Equal(t, 187766577, int(fnv32("hash-bob-5"))) - cache.AddTx(createTx([]byte("hash-carol-1"), "carol", 1)) - require.Equal(t, 3082288595, int(fnv32("hash-carol-1"))) selected, accumulatedGas := cache.SelectTransactions(math.MaxUint64, math.MaxInt) require.Len(t, selected, 8) @@ -47,10 +32,10 @@ func TestTxCache_SelectTransactions_Dummy(t *testing.T) { require.Equal(t, "hash-alice-2", string(selected[1].TxHash)) require.Equal(t, "hash-alice-3", string(selected[2].TxHash)) require.Equal(t, "hash-alice-4", string(selected[3].TxHash)) - require.Equal(t, "hash-carol-1", string(selected[4].TxHash)) - require.Equal(t, "hash-bob-5", string(selected[5].TxHash)) - require.Equal(t, "hash-bob-6", string(selected[6].TxHash)) - require.Equal(t, "hash-bob-7", string(selected[7].TxHash)) + require.Equal(t, "hash-bob-5", string(selected[4].TxHash)) + require.Equal(t, "hash-bob-6", string(selected[5].TxHash)) + require.Equal(t, "hash-bob-7", string(selected[6].TxHash)) + require.Equal(t, "hash-carol-1", string(selected[7].TxHash)) }) t.Run("alice > carol > bob", func(t *testing.T) { @@ -89,9 +74,9 @@ func TestTxCache_SelectTransactionsWithBandwidth_Dummy(t *testing.T) { require.Equal(t, 750000, int(accumulatedGas)) // Check order - require.Equal(t, "hash-carol-1", string(selected[0].TxHash)) - require.Equal(t, "hash-bob-5", string(selected[1].TxHash)) - require.Equal(t, "hash-bob-6", string(selected[2].TxHash)) + require.Equal(t, "hash-bob-5", string(selected[0].TxHash)) + require.Equal(t, "hash-bob-6", string(selected[1].TxHash)) + require.Equal(t, "hash-carol-1", string(selected[2].TxHash)) require.Equal(t, "hash-alice-1", string(selected[3].TxHash)) require.Equal(t, "hash-bob-7", string(selected[4].TxHash)) }) diff --git a/txcache/wrappedTransaction.go b/txcache/wrappedTransaction.go index a8968717..d5d652fb 100644 --- a/txcache/wrappedTransaction.go +++ b/txcache/wrappedTransaction.go @@ -1,6 +1,7 @@ package txcache import ( + "bytes" "sync/atomic" "github.com/multiversx/mx-chain-core-go/data" @@ -18,7 +19,6 @@ type WrappedTransaction struct { Size int64 PricePerUnit atomic.Uint64 - HashFnv32 atomic.Uint32 } // precomputeFields computes (and caches) the (average) price per gas unit. @@ -31,18 +31,6 @@ func (wrappedTx *WrappedTransaction) precomputeFields(txGasHandler TxGasHandler) } wrappedTx.PricePerUnit.Store(fee / gasLimit) - wrappedTx.HashFnv32.Store(fnv32(string(wrappedTx.TxHash))) -} - -// fnv32 implements https://en.wikipedia.org/wiki/Fowler–Noll–Vo_hash_function for 32 bits -func fnv32(key string) uint32 { - hash := uint32(2166136261) - const prime32 = uint32(16777619) - for i := 0; i < len(key); i++ { - hash *= prime32 - hash ^= uint32(key[i]) - } - return hash } // Equality is out of scope (not possible in our case). @@ -54,6 +42,6 @@ func (wrappedTx *WrappedTransaction) isTransactionMoreValuableForNetwork(otherTr return ppu > ppuOther } - // In the end, compare by hash number of transaction hash - return wrappedTx.HashFnv32.Load() > otherTransaction.HashFnv32.Load() + // In the end, compare by transaction hash + return bytes.Compare(wrappedTx.TxHash, otherTransaction.TxHash) < 0 } diff --git a/txcache/wrappedTransaction_test.go b/txcache/wrappedTransaction_test.go index 67a13695..b24dbc3f 100644 --- a/txcache/wrappedTransaction_test.go +++ b/txcache/wrappedTransaction_test.go @@ -15,7 +15,6 @@ func TestWrappedTransaction_precomputeFields(t *testing.T) { tx.precomputeFields(txGasHandler) require.Equal(t, oneBillion, int(tx.PricePerUnit.Load())) - require.Equal(t, 84696446, int(tx.HashFnv32.Load())) }) t.Run("move balance gas limit and execution gas limit (1)", func(t *testing.T) { @@ -23,7 +22,6 @@ func TestWrappedTransaction_precomputeFields(t *testing.T) { tx.precomputeFields(txGasHandler) require.Equal(t, 999_980_777, int(tx.PricePerUnit.Load())) - require.Equal(t, 84696445, int(tx.HashFnv32.Load())) }) t.Run("move balance gas limit and execution gas limit (2)", func(t *testing.T) { @@ -32,9 +30,7 @@ func TestWrappedTransaction_precomputeFields(t *testing.T) { actualFee := 51500*oneBillion + (oneMilion-51500)*oneBillion/100 require.Equal(t, 60_985_000_000_000, actualFee) - require.Equal(t, actualFee/oneMilion, int(tx.PricePerUnit.Load())) - require.Equal(t, 84696444, int(tx.HashFnv32.Load())) }) } @@ -54,11 +50,9 @@ func TestWrappedTransaction_isTransactionMoreValuableForNetwork(t *testing.T) { t.Run("decide by transaction hash (set them up to have the same PPU)", func(t *testing.T) { a := createTx([]byte("a-7"), "a", 7) a.precomputeFields(txGasHandler) - require.Equal(t, 2191299170, int(a.HashFnv32.Load())) b := createTx([]byte("b-7"), "b", 7) b.precomputeFields(txGasHandler) - require.Equal(t, 1654268265, int(b.HashFnv32.Load())) require.True(t, a.isTransactionMoreValuableForNetwork(b)) })