Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Use exp/slices and exp/maps to simplify some code #5479

Merged
merged 13 commits into from
Jun 22, 2023
23 changes: 5 additions & 18 deletions agreement/agreementtest/simulate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/algorand/go-deadlock"
"github.com/stretchr/testify/require"
"golang.org/x/exp/maps"

"github.com/algorand/go-algorand/agreement"
"github.com/algorand/go-algorand/config"
Expand Down Expand Up @@ -124,24 +125,10 @@ func makeTestLedger(state map[basics.Address]basics.AccountData) agreement.Ledge
func (l *testLedger) copy() *testLedger {
dup := new(testLedger)

dup.entries = make(map[basics.Round]bookkeeping.Block)
dup.certs = make(map[basics.Round]agreement.Certificate)
dup.state = make(map[basics.Address]basics.AccountData)
dup.notifications = make(map[basics.Round]signal)

for k, v := range l.entries {
dup.entries[k] = v
}
for k, v := range l.certs {
dup.certs[k] = v
}
for k, v := range l.state {
dup.state[k] = v
}
for k, v := range dup.notifications {
// note that old opened channels will now fire when these are closed
dup.notifications[k] = v
}
dup.entries = maps.Clone(l.entries)
dup.certs = maps.Clone(l.certs)
dup.state = maps.Clone(l.state)
dup.notifications = maps.Clone(l.notifications) // old opened channels will now fire when these are closed
dup.nextRound = l.nextRound

return dup
Expand Down
5 changes: 2 additions & 3 deletions agreement/autopsy.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/algorand/go-algorand/data/basics"
"github.com/algorand/go-algorand/logging"
"github.com/algorand/go-algorand/protocol"
"golang.org/x/exp/slices"
)

// An Autopsy is a trace of the ordered input events and output
Expand Down Expand Up @@ -102,9 +103,7 @@ func (m *multiCloser) Close() error {

// makeMultiCloser returns a Closer that closes all the given closers.
func makeMultiCloser(closers ...io.Closer) io.Closer {
r := make([]io.Closer, len(closers))
copy(r, closers)
return &multiCloser{r}
return &multiCloser{slices.Clone(closers)}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not even sure this needs the clone. When called as varargs, the slice is surely unshared.

}

type autopsyTrace struct {
Expand Down
16 changes: 4 additions & 12 deletions agreement/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/algorand/go-deadlock"
"github.com/stretchr/testify/require"
"golang.org/x/exp/maps"

"github.com/algorand/go-algorand/config"
"github.com/algorand/go-algorand/crypto"
Expand Down Expand Up @@ -207,10 +208,7 @@ func makeTestLedger(state map[basics.Address]basics.AccountData) Ledger {
l.certs = make(map[basics.Round]Certificate)
l.nextRound = 1

l.state = make(map[basics.Address]basics.AccountData)
for k, v := range state {
l.state[k] = v
}
l.state = maps.Clone(state)

l.notifications = make(map[basics.Round]signal)

Expand All @@ -226,10 +224,7 @@ func makeTestLedgerWithConsensusVersion(state map[basics.Address]basics.AccountD
l.certs = make(map[basics.Round]Certificate)
l.nextRound = 1

l.state = make(map[basics.Address]basics.AccountData)
for k, v := range state {
l.state[k] = v
}
l.state = maps.Clone(state)

l.notifications = make(map[basics.Round]signal)

Expand All @@ -245,10 +240,7 @@ func makeTestLedgerMaxBlocks(state map[basics.Address]basics.AccountData, maxNum

l.maxNumBlocks = maxNumBlocks

l.state = make(map[basics.Address]basics.AccountData)
for k, v := range state {
l.state[k] = v
}
l.state = maps.Clone(state)

l.notifications = make(map[basics.Round]signal)

Expand Down
6 changes: 2 additions & 4 deletions cmd/algokey/keyreg.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

"github.com/spf13/cobra"
"golang.org/x/exp/maps"

"github.com/algorand/go-algorand/crypto"
"github.com/algorand/go-algorand/data/account"
Expand Down Expand Up @@ -94,10 +95,7 @@ func init() {
"betanet": mustConvertB64ToDigest("mFgazF+2uRS1tMiL9dsj01hJGySEmPN28B/TjjvpVW0="),
"devnet": mustConvertB64ToDigest("sC3P7e2SdbqKJK0tbiCdK9tdSpbe6XeCGKdoNzmlj0E="),
}
validNetworkList = make([]string, 0, len(validNetworks))
for k := range validNetworks {
validNetworkList = append(validNetworkList, k)
}
validNetworkList = maps.Keys(validNetworks)
}

func mustConvertB64ToDigest(b64 string) (digest crypto.Digest) {
Expand Down
13 changes: 5 additions & 8 deletions cmd/goal/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"time"

"github.com/spf13/cobra"
"golang.org/x/exp/slices"

"github.com/algorand/go-algorand/cmd/util/datadir"
"github.com/algorand/go-algorand/config"
Expand Down Expand Up @@ -557,35 +558,31 @@ var infoCmd = &cobra.Command{
func printAccountInfo(client libgoal.Client, address string, onlyShowAssetIds bool, account model.Account) bool {
var createdAssets []model.Asset
if account.CreatedAssets != nil {
createdAssets = make([]model.Asset, len(*account.CreatedAssets))
copy(createdAssets, *account.CreatedAssets)
createdAssets = slices.Clone(*account.CreatedAssets)
sort.Slice(createdAssets, func(i, j int) bool {
return createdAssets[i].Index < createdAssets[j].Index
})
}

var heldAssets []model.AssetHolding
if account.Assets != nil {
heldAssets = make([]model.AssetHolding, len(*account.Assets))
copy(heldAssets, *account.Assets)
heldAssets = slices.Clone(*account.Assets)
sort.Slice(heldAssets, func(i, j int) bool {
return heldAssets[i].AssetID < heldAssets[j].AssetID
})
}

var createdApps []model.Application
if account.CreatedApps != nil {
createdApps = make([]model.Application, len(*account.CreatedApps))
copy(createdApps, *account.CreatedApps)
createdApps = slices.Clone(*account.CreatedApps)
sort.Slice(createdApps, func(i, j int) bool {
return createdApps[i].Id < createdApps[j].Id
})
}

var optedInApps []model.ApplicationLocalState
if account.AppsLocalState != nil {
optedInApps = make([]model.ApplicationLocalState, len(*account.AppsLocalState))
copy(optedInApps, *account.AppsLocalState)
optedInApps = slices.Clone(*account.AppsLocalState)
sort.Slice(optedInApps, func(i, j int) bool {
return optedInApps[i].Id < optedInApps[j].Id
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/opdoc/opdoc.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ func main() {
AVMType: t.AVMType.String(),
})
}
sort.Slice(named, func(i, j int) bool { return strings.Compare(named[i].Name, named[j].Name) > 0 })
sort.Slice(named, func(i, j int) bool { return named[i].Name > named[j].Name })

namedStackTypes := create("named_stack_types.md")
namedStackTypesMarkdown(namedStackTypes, named)
Expand Down
4 changes: 2 additions & 2 deletions cmd/tealdbg/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/algorand/go-algorand/data/transactions/logic"
"github.com/algorand/go-algorand/ledger/apply"
"github.com/algorand/go-algorand/protocol"
"golang.org/x/exp/slices"
)

func protoFromString(protoString string) (name string, proto config.ConsensusParams, err error) {
Expand Down Expand Up @@ -190,8 +191,7 @@ func (a *AppState) clone() (b AppState) {
b.locals[addr][aid] = tkv.Clone()
}
}
b.logs = make([]string, len(a.logs))
copy(b.logs, a.logs)
b.logs = slices.Clone(a.logs)
b.innerTxns = cloneInners(a.innerTxns)
return
}
Expand Down
3 changes: 2 additions & 1 deletion crypto/merklearray/merkle.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"sort"

"github.com/algorand/go-algorand/crypto"
"golang.org/x/exp/slices"
)

const (
Expand Down Expand Up @@ -223,7 +224,7 @@ func (tree *Tree) Prove(idxs []uint64) (*Proof, error) {
idxs = VcIdxs
}

sort.Slice(idxs, func(i, j int) bool { return idxs[i] < idxs[j] })
slices.Sort(idxs)

return tree.createProof(idxs)
}
Expand Down
11 changes: 5 additions & 6 deletions crypto/merkletrie/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (
"encoding/binary"
"errors"
"fmt"
"sort"

"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
)

// storedNodeIdentifier is the "equivalent" of a node-ptr, but oriented around persisting the
Expand Down Expand Up @@ -446,11 +448,8 @@ func (mtc *merkleTrieCache) reallocatePendingPages(stats *CommitStats) (pagesToC
}

// create a sorted list of created pages
sortedCreatedPages := make([]uint64, 0, len(createdPages))
for page := range createdPages {
sortedCreatedPages = append(sortedCreatedPages, page)
}
sort.SliceStable(sortedCreatedPages, func(i, j int) bool { return sortedCreatedPages[i] < sortedCreatedPages[j] })
sortedCreatedPages := maps.Keys(createdPages)
slices.Sort(sortedCreatedPages)
jasonpaulos marked this conversation as resolved.
Show resolved Hide resolved

mtc.reallocatedPages = make(map[uint64]map[storedNodeIdentifier]*node)

Expand Down
6 changes: 3 additions & 3 deletions crypto/merkletrie/committer.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package merkletrie

import "golang.org/x/exp/slices"

// Committer is the interface supporting serializing tries into persistent storage.
type Committer interface {
StorePage(page uint64, content []byte) error
Expand All @@ -40,9 +42,7 @@ func (mc *InMemoryCommitter) StorePage(page uint64, content []byte) error {
if content == nil {
delete(mc.memStore, page)
} else {
storedContent := make([]byte, len(content))
copy(storedContent, content)
mc.memStore[page] = storedContent
mc.memStore[page] = slices.Clone(content)
}
return nil
}
Expand Down
5 changes: 2 additions & 3 deletions crypto/merkletrie/committer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/stretchr/testify/require"
"golang.org/x/exp/slices"

"github.com/algorand/go-algorand/crypto"
"github.com/algorand/go-algorand/test/partitiontest"
Expand All @@ -33,9 +34,7 @@ func (mc *InMemoryCommitter) Duplicate(flat bool) (out *InMemoryCommitter) {
if flat {
out.memStore[k] = v
} else {
bytes := make([]byte, len(v))
copy(bytes[:], v[:])
out.memStore[k] = bytes
out.memStore[k] = slices.Clone(v)
jannotti marked this conversation as resolved.
Show resolved Hide resolved
}
}
return
Expand Down
4 changes: 2 additions & 2 deletions crypto/merkletrie/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"unsafe"

"github.com/algorand/go-algorand/crypto"
"golang.org/x/exp/slices"
)

type childEntry struct {
Expand Down Expand Up @@ -339,8 +340,7 @@ func deserializeNode(buf []byte) (n *node, s int) {
if hashLength2 <= 0 {
return nil, hashLength2
}
n.hash = make([]byte, hashLength)
copy(n.hash, buf[hashLength2:hashLength2+int(hashLength)])
n.hash = slices.Clone(buf[hashLength2 : hashLength2+int(hashLength)])
s = hashLength2 + int(hashLength)
isLeaf := (buf[s] == 0)
s++
Expand Down
4 changes: 2 additions & 2 deletions daemon/algod/api/server/v2/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/algorand/go-algorand/crypto"
"github.com/algorand/go-algorand/daemon/algod/api/server/v2/generated/model"
"github.com/algorand/go-algorand/data/basics"
"golang.org/x/exp/slices"
)

// AssetHolding converts between basics.AssetHolding and model.AssetHolding
Expand Down Expand Up @@ -477,8 +478,7 @@ func AssetParamsToAsset(creator string, idx basics.AssetIndex, params *basics.As
Reserve: addrOrNil(params.Reserve),
}
if params.MetadataHash != ([32]byte{}) {
metadataHash := make([]byte, len(params.MetadataHash))
copy(metadataHash, params.MetadataHash[:])
metadataHash := slices.Clone(params.MetadataHash[:])
assetParams.MetadataHash = &metadataHash
}

Expand Down
5 changes: 3 additions & 2 deletions daemon/algod/api/server/v2/test/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
"github.com/algorand/go-algorand/daemon/algod/api/server"
"github.com/algorand/go-algorand/ledger/eval"
"github.com/algorand/go-algorand/ledger/ledgercore"
"golang.org/x/exp/slices"

"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -1029,8 +1031,7 @@ int 1`,

var expectedFailedAt *[]uint64
if len(scenario.FailedAt) != 0 {
clone := make([]uint64, len(scenario.FailedAt))
copy(clone, scenario.FailedAt)
clone := slices.Clone(scenario.FailedAt)
clone[0]++
expectedFailedAt = &clone
}
Expand Down
4 changes: 2 additions & 2 deletions daemon/algod/api/server/v2/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/algorand/go-codec/codec"
"github.com/labstack/echo/v4"
"golang.org/x/exp/slices"

"github.com/algorand/go-algorand/daemon/algod/api/server/v2/generated/model"
"github.com/algorand/go-algorand/data/basics"
Expand Down Expand Up @@ -431,8 +432,7 @@ func convertTxnGroupResult(txnGroupResult simulation.TxnGroupResult) PreEncodedS
}

if len(txnGroupResult.FailedAt) > 0 {
failedAt := make([]uint64, len(txnGroupResult.FailedAt))
copy(failedAt, txnGroupResult.FailedAt)
failedAt := slices.Clone[[]uint64, uint64](txnGroupResult.FailedAt)
encoded.FailedAt = &failedAt
}

Expand Down
29 changes: 3 additions & 26 deletions data/basics/teal.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"

"github.com/algorand/go-algorand/config"
"golang.org/x/exp/maps"
)

// DeltaAction is an enum of actions that may be performed when applying a
Expand Down Expand Up @@ -77,24 +78,7 @@ type StateDelta map[string]ValueDelta
// equality because an empty map will encode/decode as nil. So if our generated
// map is empty but not nil, we want to equal a decoded nil off the wire.
func (sd StateDelta) Equal(o StateDelta) bool {
// Lengths should be the same
if len(sd) != len(o) {
return false
}
// All keys and deltas should be the same
for k, v := range sd {
// Other StateDelta must contain key
ov, ok := o[k]
if !ok {
return false
}

// Other StateDelta must have same value for key
if ov != v {
return false
}
}
return true
return maps.Equal(sd, o)
Copy link
Contributor Author

@jannotti jannotti Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment goes into a lot of detail about the nil checking choice, but that is exactly the same choice that maps.Equal made. Unit tests already exist to confirm.

}

// Valid checks whether the keys and values in a StateDelta conform to the
Expand Down Expand Up @@ -234,14 +218,7 @@ type TealKeyValue map[string]TealValue
// Clone returns a copy of a TealKeyValue that may be modified without
// affecting the original
func (tk TealKeyValue) Clone() TealKeyValue {
if tk == nil {
return nil
}
res := make(TealKeyValue, len(tk))
for k, v := range tk {
res[k] = v
}
return res
return maps.Clone(tk)
}

// ToStateSchema calculates the number of each value type in a TealKeyValue and
Expand Down
Loading