From 204790f4c70e198cc06fe54e9205a71567ca6c83 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Tue, 29 Jun 2021 21:09:45 -0600 Subject: [PATCH] fix(vbank): ensure that multiple balance updates are sorted --- go.sum | 1 + golang/cosmos/x/vbank/vbank.go | 39 ++++++++++++++++++++++++++--- golang/cosmos/x/vbank/vbank_test.go | 19 +++++++++----- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/go.sum b/go.sum index 2bdf8506383..29b08f787c6 100644 --- a/go.sum +++ b/go.sum @@ -293,6 +293,7 @@ github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/mock v1.4.4 h1:l75CXGRSwbaYNpl/Z2X1XIIAMSCquvXgpVZDhwEIJsc= github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4= +github.com/golang/mock v1.5.0 h1:jlYHihg//f7RRwuPfptm04yp4s7O6Kw8EZiVYIGcH0g= github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.0/go.mod h1:Qd/q+1AKNOZr9uGQzbzCmRO6sUih6GTPZv6a1/R87v0= diff --git a/golang/cosmos/x/vbank/vbank.go b/golang/cosmos/x/vbank/vbank.go index e5d11a8fd64..e2e95c83dae 100644 --- a/golang/cosmos/x/vbank/vbank.go +++ b/golang/cosmos/x/vbank/vbank.go @@ -3,6 +3,7 @@ package vbank import ( "encoding/json" "fmt" + "sort" "github.com/Agoric/agoric-sdk/golang/cosmos/vm" sdk "github.com/cosmos/cosmos-sdk/types" @@ -35,10 +36,37 @@ type vbankSingleBalanceUpdate struct { Amount string `json:"amount"` } +// Make vbankManyBalanceUpdates sortable +type vbankManyBalanceUpdates []vbankSingleBalanceUpdate + +var _ sort.Interface = vbankManyBalanceUpdates{} + +func (vbu vbankManyBalanceUpdates) Len() int { + return len(vbu) +} + +func (vbu vbankManyBalanceUpdates) Less(i int, j int) bool { + if vbu[i].Address < vbu[j].Address { + return true + } else if vbu[i].Address > vbu[j].Address { + return false + } + if vbu[i].Denom < vbu[j].Denom { + return true + } else if vbu[i].Denom > vbu[j].Denom { + return false + } + return vbu[i].Amount < vbu[j].Amount +} + +func (vbu vbankManyBalanceUpdates) Swap(i int, j int) { + vbu[i], vbu[j] = vbu[j], vbu[i] +} + type vbankBalanceUpdate struct { - Nonce uint64 `json:"nonce"` - Type string `json:"type"` - Updated []vbankSingleBalanceUpdate `json:"updated"` + Nonce uint64 `json:"nonce"` + Type string `json:"type"` + Updated vbankManyBalanceUpdates `json:"updated"` } func marshalBalanceUpdate(ctx sdk.Context, keeper Keeper, addressToBalance map[string]sdk.Coins) ([]byte, error) { @@ -53,6 +81,9 @@ func marshalBalanceUpdate(ctx sdk.Context, keeper Keeper, addressToBalance map[s Nonce: nonce, Updated: make([]vbankSingleBalanceUpdate, 0, nentries), } + + // Note that Golang randomises the order of iteration, so we have to sort + // below to be deterministic. for address, coins := range addressToBalance { for _, coin := range coins { update := vbankSingleBalanceUpdate{ @@ -64,6 +95,8 @@ func marshalBalanceUpdate(ctx sdk.Context, keeper Keeper, addressToBalance map[s } } + // Ensure we have a deterministic order of updates. + sort.Sort(event.Updated) return json.Marshal(&event) } diff --git a/golang/cosmos/x/vbank/vbank_test.go b/golang/cosmos/x/vbank/vbank_test.go index 57c5f0bf350..94270d341a8 100644 --- a/golang/cosmos/x/vbank/vbank_test.go +++ b/golang/cosmos/x/vbank/vbank_test.go @@ -82,7 +82,11 @@ func decodeBalances(encoded []byte) (balances, uint64, error) { return nil, 0, fmt.Errorf("bad balance update type: %s", balanceUpdate.Type) } b := newBalances() - for _, u := range balanceUpdate.Updated { + fmt.Printf("updated balances %v\n", balanceUpdate.Updated) + for i, u := range balanceUpdate.Updated { + if i < len(balanceUpdate.Updated)-1 && balanceUpdate.Updated.Less(i+1, i) { + return nil, 0, fmt.Errorf("unordered balance update %v is before %v", u, balanceUpdate.Updated[i+1]) + } account(u.Address, coin(u.Denom, u.Amount))(b) } return b, balanceUpdate.Nonce, nil @@ -99,6 +103,7 @@ func Test_marshalBalanceUpdate(t *testing.T) { addressToBalance map[string]sdk.Coins want balances wantErr bool + encoded []byte }{ { name: "empty", @@ -277,8 +282,9 @@ func Test_Receive_Give(t *testing.T) { want := newBalances(account(addr1, coin("urun", "1000"))) got, gotNonce, err := decodeBalances([]byte(ret)) if err != nil { - t.Errorf("decode balances error = %v", err) - } else if !reflect.DeepEqual(got, want) { + t.Fatalf("decode balances error = %v", err) + } + if !reflect.DeepEqual(got, want) { t.Errorf("got %+v, want %+v", got, want) } nonce := uint64(1) @@ -418,8 +424,9 @@ func Test_Receive_Grab(t *testing.T) { want := newBalances(account(addr1, coin("ubld", "1000"))) got, gotNonce, err := decodeBalances([]byte(ret)) if err != nil { - t.Errorf("decode balances error = %v", err) - } else if !reflect.DeepEqual(got, want) { + t.Fatalf("decode balances error = %v", err) + } + if !reflect.DeepEqual(got, want) { t.Errorf("got %+v, want %+v", got, want) } nonce := uint64(1) @@ -495,7 +502,7 @@ func Test_EndBlock_Events(t *testing.T) { } gotMsg, gotNonce, err := decodeBalances([]byte(msgsSent[0])) if err != nil { - t.Errorf("decode balances error = %v", err) + t.Fatalf("decode balances error = %v", err) } nonce := uint64(1)