Skip to content

Commit

Permalink
Use avalanchego's sorting methods (ava-labs#260)
Browse files Browse the repository at this point in the history
* use avalanchgo sorting functions

* add tests for Less

* import ordering

* import ordering

* Use `slices` methods instead of `sort` methods (ava-labs#261)

* use slices.Sort instead of sort.Slice

* import ordering

* add test case
  • Loading branch information
Dan Laine authored Jun 28, 2023
1 parent 3f0df72 commit fbd9dad
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 66 deletions.
9 changes: 6 additions & 3 deletions plugin/evm/atomic_tx_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import (
"encoding/binary"
"errors"
"fmt"
"sort"
"time"

"golang.org/x/exp/slices"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/log"

Expand Down Expand Up @@ -272,7 +273,9 @@ func (a *atomicTxRepository) write(height uint64, txs []*Tx, bonus bool) error {
// with txs initialized from the txID index.
copyTxs := make([]*Tx, len(txs))
copy(copyTxs, txs)
sort.Slice(copyTxs, func(i, j int) bool { return copyTxs[i].ID().Hex() < copyTxs[j].ID().Hex() })
slices.SortFunc(copyTxs, func(i, j *Tx) bool {
return i.Less(j)
})
txs = copyTxs
}
heightBytes := make([]byte, wrappers.LongLen)
Expand Down Expand Up @@ -452,6 +455,6 @@ func getAtomicRepositoryRepairHeights(bonusBlocks map[uint64]ids.ID, canonicalBl
repairHeights = append(repairHeights, height)
}
}
sort.Slice(repairHeights, func(i, j int) bool { return repairHeights[i] < repairHeights[j] })
slices.Sort(repairHeights)
return repairHeights
}
12 changes: 5 additions & 7 deletions plugin/evm/atomic_tx_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ package evm
import (
"encoding/binary"
"fmt"
"sort"
"testing"

"golang.org/x/exp/slices"

"github.com/ava-labs/avalanchego/chains/atomic"
"github.com/ava-labs/avalanchego/database"
"github.com/ava-labs/avalanchego/database/prefixdb"
Expand Down Expand Up @@ -98,17 +99,14 @@ func writeTxs(t testing.TB, repo AtomicTxRepository, fromHeight uint64, toHeight
// verifyTxs asserts [repo] can find all txs in [txMap] by height and txID
func verifyTxs(t testing.TB, repo AtomicTxRepository, txMap map[uint64][]*Tx) {
// We should be able to fetch indexed txs by height:
getComparator := func(txs []*Tx) func(int, int) bool {
return func(i, j int) bool {
return txs[i].ID().Hex() < txs[j].ID().Hex()
}
}
for height, expectedTxs := range txMap {
txs, err := repo.GetByHeight(height)
assert.NoErrorf(t, err, "unexpected error on GetByHeight at height=%d", height)
assert.Lenf(t, txs, len(expectedTxs), "wrong len of txs at height=%d", height)
// txs should be stored in order of txID
sort.Slice(expectedTxs, getComparator(expectedTxs))
slices.SortFunc(expectedTxs, func(i, j *Tx) bool {
return i.Less(j)
})

txIDs := set.Set[ids.ID]{}
for i := 0; i < len(txs); i++ {
Expand Down
3 changes: 2 additions & 1 deletion plugin/evm/export_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/ava-labs/avalanchego/chains/atomic"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
"github.com/ava-labs/avalanchego/utils/math"
Expand Down Expand Up @@ -117,7 +118,7 @@ func (utx *UnsignedExportTx) Verify(
if !avax.IsSortedTransferableOutputs(utx.ExportedOutputs, Codec) {
return errOutputsNotSorted
}
if rules.IsApricotPhase1 && !IsSortedAndUniqueEVMInputs(utx.Ins) {
if rules.IsApricotPhase1 && !utils.IsSortedAndUniqueSortable(utx.Ins) {
return errInputsNotSortedUnique
}

Expand Down
10 changes: 7 additions & 3 deletions plugin/evm/import_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"fmt"
"math/big"

"golang.org/x/exp/slices"

"github.com/ava-labs/coreth/core/state"
"github.com/ava-labs/coreth/params"

Expand Down Expand Up @@ -110,11 +112,13 @@ func (utx *UnsignedImportTx) Verify(
}

if rules.IsApricotPhase2 {
if !IsSortedAndUniqueEVMOutputs(utx.Outs) {
if !utils.IsSortedAndUniqueSortable(utx.Outs) {
return errOutputsNotSortedUnique
}
} else if rules.IsApricotPhase1 {
if !IsSortedEVMOutputs(utx.Outs) {
if !slices.IsSortedFunc(utx.Outs, func(i, j EVMOutput) bool {
return i.Less(j)
}) {
return errOutputsNotSorted
}
}
Expand Down Expand Up @@ -408,7 +412,7 @@ func (vm *VM) newImportTxWithUTXOs(
return nil, errNoEVMOutputs
}

SortEVMOutputs(outs)
utils.Sort(outs)

// Create the transaction
utx := &UnsignedImportTx{
Expand Down
2 changes: 1 addition & 1 deletion plugin/evm/import_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestImportTxVerify(t *testing.T) {

// Sort the inputs and outputs to ensure the transaction is canonical
utils.Sort(importTx.ImportedInputs)
SortEVMOutputs(importTx.Outs)
utils.Sort(importTx.Outs)

tests := map[string]atomicTxVerifyTest{
"nil tx": {
Expand Down
2 changes: 1 addition & 1 deletion plugin/evm/mempool_atomic_gossiping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func createImportTx(t *testing.T, vm *VM, txID ids.ID, feeAmount uint64) *Tx {

// Sort the inputs and outputs to ensure the transaction is canonical
utils.Sort(importTx.ImportedInputs)
SortEVMOutputs(importTx.Outs)
utils.Sort(importTx.Outs)

tx := &Tx{UnsignedAtomicTx: importTx}
// Sign with the correct key
Expand Down
70 changes: 25 additions & 45 deletions plugin/evm/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"math/big"
"sort"

"golang.org/x/exp/slices"

"github.com/ethereum/go-ethereum/common"

"github.com/ava-labs/coreth/core/state"
Expand All @@ -19,7 +21,6 @@ import (
"github.com/ava-labs/avalanchego/codec"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/crypto/secp256k1"
"github.com/ava-labs/avalanchego/utils/hashing"
"github.com/ava-labs/avalanchego/utils/set"
Expand Down Expand Up @@ -55,6 +56,14 @@ type EVMOutput struct {
AssetID ids.ID `serialize:"true" json:"assetID"`
}

func (o EVMOutput) Less(other EVMOutput) bool {
addrComp := bytes.Compare(o.Address.Bytes(), other.Address.Bytes())
if addrComp != 0 {
return addrComp < 0
}
return bytes.Compare(o.AssetID[:], other.AssetID[:]) < 0
}

// EVMInput defines an input created from the EVM state to fund export transactions
type EVMInput struct {
Address common.Address `serialize:"true" json:"address"`
Expand All @@ -63,6 +72,14 @@ type EVMInput struct {
Nonce uint64 `serialize:"true" json:"nonce"`
}

func (i EVMInput) Less(other EVMInput) bool {
addrComp := bytes.Compare(i.Address.Bytes(), other.Address.Bytes())
if addrComp != 0 {
return addrComp < 0
}
return bytes.Compare(i.AssetID[:], other.AssetID[:]) < 0
}

// Verify ...
func (out *EVMOutput) Verify() error {
switch {
Expand Down Expand Up @@ -126,6 +143,10 @@ type Tx struct {
Creds []verify.Verifiable `serialize:"true" json:"credentials"`
}

func (tx *Tx) Less(other *Tx) bool {
return tx.ID().Hex() < other.ID().Hex()
}

// Sign this transaction with the provided signers
func (tx *Tx) Sign(c codec.Manager, signers [][]*secp256k1.PrivateKey) error {
unsignedBytes, err := c.Marshal(codecVersion, &tx.UnsignedAtomicTx)
Expand Down Expand Up @@ -217,49 +238,6 @@ func SortEVMInputsAndSigners(inputs []EVMInput, signers [][]*secp256k1.PrivateKe
sort.Sort(&innerSortInputsAndSigners{inputs: inputs, signers: signers})
}

// IsSortedAndUniqueEVMInputs returns true if the EVM Inputs are sorted and unique
// based on the account addresses
func IsSortedAndUniqueEVMInputs(inputs []EVMInput) bool {
return utils.IsSortedAndUnique(&innerSortInputsAndSigners{inputs: inputs})
}

// innerSortEVMOutputs implements sort.Interface for EVMOutput
type innerSortEVMOutputs struct {
outputs []EVMOutput
}

func (outs *innerSortEVMOutputs) Less(i, j int) bool {
addrComp := bytes.Compare(outs.outputs[i].Address.Bytes(), outs.outputs[j].Address.Bytes())
if addrComp != 0 {
return addrComp < 0
}
return bytes.Compare(outs.outputs[i].AssetID[:], outs.outputs[j].AssetID[:]) < 0
}

func (outs *innerSortEVMOutputs) Len() int { return len(outs.outputs) }

func (outs *innerSortEVMOutputs) Swap(i, j int) {
outs.outputs[j], outs.outputs[i] = outs.outputs[i], outs.outputs[j]
}

// SortEVMOutputs sorts the list of EVMOutputs based on the addresses and assetIDs
// of the outputs
func SortEVMOutputs(outputs []EVMOutput) {
sort.Sort(&innerSortEVMOutputs{outputs: outputs})
}

// IsSortedEVMOutputs returns true if the EVMOutputs are sorted
// based on the account addresses and assetIDs
func IsSortedEVMOutputs(outputs []EVMOutput) bool {
return sort.IsSorted(&innerSortEVMOutputs{outputs: outputs})
}

// IsSortedAndUniqueEVMOutputs returns true if the EVMOutputs are sorted
// and unique based on the account addresses and assetIDs
func IsSortedAndUniqueEVMOutputs(outputs []EVMOutput) bool {
return utils.IsSortedAndUnique(&innerSortEVMOutputs{outputs: outputs})
}

// calculates the amount of AVAX that must be burned by an atomic transaction
// that consumes [cost] at [baseFee].
func CalculateDynamicFee(cost uint64, baseFee *big.Int) (uint64, error) {
Expand Down Expand Up @@ -289,7 +267,9 @@ func mergeAtomicOps(txs []*Tx) (map[ids.ID]*atomic.Requests, error) {
// with txs initialized from the txID index.
copyTxs := make([]*Tx, len(txs))
copy(copyTxs, txs)
sort.Slice(copyTxs, func(i, j int) bool { return copyTxs[i].ID().Hex() < copyTxs[j].ID().Hex() })
slices.SortFunc(copyTxs, func(i, j *Tx) bool {
return i.Less(j)
})
txs = copyTxs
}
output := make(map[ids.ID]*atomic.Requests)
Expand Down
Loading

0 comments on commit fbd9dad

Please sign in to comment.