From db9c2357fe43a3d320f3ada6d478f6a8ad77b779 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Wed, 22 Sep 2021 22:45:30 +0200 Subject: [PATCH 1/9] :truck: [wallet/test] Rename wallettest.go to wallet.go Because the file is already in directory test, it does not need to include "test" in its name. Signed-off-by: Matthias Geihs --- wallet/test/{wallettest.go => wallet.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename wallet/test/{wallettest.go => wallet.go} (100%) diff --git a/wallet/test/wallettest.go b/wallet/test/wallet.go similarity index 100% rename from wallet/test/wallettest.go rename to wallet/test/wallet.go From b9a7bb8330fc4ea978c00dfd43d4a540adc89041 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 23 Sep 2021 08:32:34 +0200 Subject: [PATCH 2/9] :fire: [wallet,backend/sim/wallet] Remove set backend test and set randomizer test There are several problems with `func TestSetBackend` and `func SetRandomizerTest`: - They look like a test function but they are contained within a non-test package. - They do not give any valuable information: We use them in the sim backend tests. However, the backend tests would fail anyways if not initialized correctly. Signed-off-by: Matthias Geihs --- backend/sim/wallet/init_test.go | 27 --------------------------- wallet/backendtest.go | 29 ----------------------------- wallet/test/randomizertest.go | 29 ----------------------------- 3 files changed, 85 deletions(-) delete mode 100644 backend/sim/wallet/init_test.go delete mode 100644 wallet/backendtest.go delete mode 100644 wallet/test/randomizertest.go diff --git a/backend/sim/wallet/init_test.go b/backend/sim/wallet/init_test.go deleted file mode 100644 index b7c18370..00000000 --- a/backend/sim/wallet/init_test.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2019 - See NOTICE file for copyright holders. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package wallet - -import ( - "testing" - - "perun.network/go-perun/wallet" - "perun.network/go-perun/wallet/test" -) - -func TestSetBackend(t *testing.T) { - wallet.SetBackendTest(t) - test.SetRandomizerTest(t) -} diff --git a/wallet/backendtest.go b/wallet/backendtest.go deleted file mode 100644 index 8775c137..00000000 --- a/wallet/backendtest.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2019 - See NOTICE file for copyright holders. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package wallet - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// SetBackendTest is a generic test to test that the wallet backend is set correctly. -func SetBackendTest(t *testing.T) { - assert.Panics(t, func() { SetBackend(nil) }, "nil backend set should panic") - require.NotNil(t, backend, "backend should be already set by init()") - assert.Panics(t, func() { SetBackend(backend) }, "setting a backend twice should panic") -} diff --git a/wallet/test/randomizertest.go b/wallet/test/randomizertest.go deleted file mode 100644 index fc2896ca..00000000 --- a/wallet/test/randomizertest.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2019 - See NOTICE file for copyright holders. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package test - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// SetRandomizerTest is a generic test to test that the wallet randomizer is set correctly. -func SetRandomizerTest(t *testing.T) { - assert.Panics(t, func() { SetRandomizer(nil) }, "nil backend set should panic") - require.NotNil(t, randomizer, "backend should be already set by init()") - assert.Panics(t, func() { SetRandomizer(randomizer) }, "setting a backend twice should panic") -} From 444bbe7ad7925972577ccae03cca20d743f3ab92 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Tue, 5 Oct 2021 16:05:40 +0200 Subject: [PATCH 3/9] :truck: [wallet/test] Move address test to separate file We can use one test file per interface. Signed-off-by: Matthias Geihs --- wallet/test/address.go | 52 ++++++++++++++++++++++++++++++++++++++++++ wallet/test/wallet.go | 29 ----------------------- 2 files changed, 52 insertions(+), 29 deletions(-) create mode 100644 wallet/test/address.go diff --git a/wallet/test/address.go b/wallet/test/address.go new file mode 100644 index 00000000..3d8b8c06 --- /dev/null +++ b/wallet/test/address.go @@ -0,0 +1,52 @@ +// Copyright 2019 - See NOTICE file for copyright holders. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package test + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// GenericAddressTest runs a test suite designed to test the general functionality of addresses. +// This function should be called by every implementation of the wallet interface. +func GenericAddressTest(t *testing.T, s *Setup) { + addrLen := len(s.AddressEncoded) + null, err := s.Backend.DecodeAddress(bytes.NewReader(make([]byte, addrLen))) + assert.NoError(t, err, "Byte deserialization of zero address should work") + addr, err := s.Backend.DecodeAddress(bytes.NewReader(s.AddressEncoded)) + assert.NoError(t, err, "Byte deserialization of address should work") + + nullString := null.String() + addrString := addr.String() + assert.Greater(t, len(nullString), 0) + assert.Greater(t, len(addrString), 0) + assert.NotEqual(t, addrString, nullString) + + assert.False(t, addr.Equals(null), "Expected inequality of zero, nonzero address") + assert.True(t, null.Equals(null), "Expected equality of zero address to itself") + + // a.Equals(Decode(Encode(a))) + t.Run("Serialize Equals Test", func(t *testing.T) { + buff := new(bytes.Buffer) + require.NoError(t, addr.Encode(buff)) + addr2, err := s.Backend.DecodeAddress(buff) + require.NoError(t, err) + + assert.True(t, addr.Equals(addr2)) + }) +} diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index 62700f71..362d8446 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -119,32 +119,3 @@ func GenericSignatureSizeTest(t *testing.T, s *Setup) { }) } } - -// GenericAddressTest runs a test suite designed to test the general functionality of addresses. -// This function should be called by every implementation of the wallet interface. -func GenericAddressTest(t *testing.T, s *Setup) { - addrLen := len(s.AddressEncoded) - null, err := s.Backend.DecodeAddress(bytes.NewReader(make([]byte, addrLen))) - assert.NoError(t, err, "Byte deserialization of zero address should work") - addr, err := s.Backend.DecodeAddress(bytes.NewReader(s.AddressEncoded)) - assert.NoError(t, err, "Byte deserialization of address should work") - - nullString := null.String() - addrString := addr.String() - assert.Greater(t, len(nullString), 0) - assert.Greater(t, len(addrString), 0) - assert.NotEqual(t, addrString, nullString) - - assert.False(t, addr.Equals(null), "Expected inequality of zero, nonzero address") - assert.True(t, null.Equals(null), "Expected equality of zero address to itself") - - // a.Equals(Decode(Encode(a))) - t.Run("Serialize Equals Test", func(t *testing.T) { - buff := new(bytes.Buffer) - require.NoError(t, addr.Encode(buff)) - addr2, err := s.Backend.DecodeAddress(buff) - require.NoError(t, err) - - assert.True(t, addr.Equals(addr2)) - }) -} From f443ea2ff7f2035a5c2cb5a8d3e7c1405d07da7a Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 23 Sep 2021 09:06:07 +0200 Subject: [PATCH 4/9] :white_check_mark: [wallet/test] Extend address test Signed-off-by: Matthias Geihs --- wallet/test/address.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/wallet/test/address.go b/wallet/test/address.go index 3d8b8c06..ac78885c 100644 --- a/wallet/test/address.go +++ b/wallet/test/address.go @@ -31,15 +31,23 @@ func GenericAddressTest(t *testing.T, s *Setup) { addr, err := s.Backend.DecodeAddress(bytes.NewReader(s.AddressEncoded)) assert.NoError(t, err, "Byte deserialization of address should work") + // Test Address.String. nullString := null.String() addrString := addr.String() assert.Greater(t, len(nullString), 0) assert.Greater(t, len(addrString), 0) assert.NotEqual(t, addrString, nullString) + // Test Address.Equals. assert.False(t, addr.Equals(null), "Expected inequality of zero, nonzero address") assert.True(t, null.Equals(null), "Expected equality of zero address to itself") + // Test Address.Bytes. + addrBytes := addr.Bytes() + nullBytes := null.Bytes() + assert.False(t, bytes.Equal(addrBytes, nullBytes), "Expected inequality of byte representations of nonzero and zero address") + assert.True(t, bytes.Equal(addrBytes, addr.Bytes()), "Expected that byte representations do not change") + // a.Equals(Decode(Encode(a))) t.Run("Serialize Equals Test", func(t *testing.T) { buff := new(bytes.Buffer) From 369d6b06de1a5057eaba3155baef56be8eda0a2f Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 23 Sep 2021 09:29:30 +0200 Subject: [PATCH 5/9] :art: [wallet/test,backend/.../wallet] Revise test.Setup data struct - Wallet should be provided with setup - UnlockedAccount was not provided an unlocked account, but calling Wallet.Unlock, which made it cumbersome to use. - Provide account address of wallet account instead of unlocked account Signed-off-by: Matthias Geihs --- backend/ethereum/wallet/hd/wallet_test.go | 17 ++++----- .../ethereum/wallet/keystore/wallet_test.go | 13 ++++--- backend/ethereum/wallet/simple/wallet_test.go | 17 ++++----- backend/sim/wallet/wallet_internal_test.go | 20 ++++++---- wallet/test/wallet.go | 37 ++++++++++--------- wallet/test/walletbench.go | 4 +- 6 files changed, 56 insertions(+), 52 deletions(-) diff --git a/backend/ethereum/wallet/hd/wallet_test.go b/backend/ethereum/wallet/hd/wallet_test.go index 81cf503c..4fbba558 100644 --- a/backend/ethereum/wallet/hd/wallet_test.go +++ b/backend/ethereum/wallet/hd/wallet_test.go @@ -28,7 +28,6 @@ import ( ethwallet "perun.network/go-perun/backend/ethereum/wallet" "perun.network/go-perun/backend/ethereum/wallet/hd" pkgtest "perun.network/go-perun/pkg/test" - "perun.network/go-perun/wallet" "perun.network/go-perun/wallet/test" ) @@ -87,8 +86,7 @@ func TestUnlock(t *testing.T) { _, err := hdWallet.Unlock(ethwallet.AsWalletAddr(missingAddr)) assert.Error(t, err, "should error on unlocking missing address") - validAcc, _ := setup.UnlockedAccount() - acc, err := hdWallet.Unlock(validAcc.Address()) + acc, err := hdWallet.Unlock(setup.Address) assert.NoError(t, err, "should not error on unlocking valid address") assert.NotNil(t, acc, "account should be non nil when error is nil") } @@ -101,9 +99,7 @@ func TestContains(t *testing.T) { missingAddr := common.BytesToAddress(setup.AddressEncoded) assert.False(t, hdWallet.Contains(missingAddr), "should not contain address of the missing account") - validAcc, err := setup.UnlockedAccount() - require.NoError(t, err) - assert.True(t, hdWallet.Contains(ethwallet.AsEthAddr(validAcc.Address())), "should contain valid account") + assert.True(t, hdWallet.Contains(ethwallet.AsEthAddr(setup.Address)), "should contain valid account") } // nolint:interfacer // rand.Rand is preferred over io.Reader here. @@ -129,9 +125,10 @@ func newSetup(t require.TestingT, prng *rand.Rand) (*test.Setup, accounts.Wallet require.NoError(t, err, "invalid sample address") return &test.Setup{ - UnlockedAccount: func() (wallet.Account, error) { return acc, nil }, - Backend: new(ethwallet.Backend), - AddressEncoded: sampleBytes, - DataToSign: dataToSign, + Wallet: hdWallet, + Address: acc.Address(), + Backend: new(ethwallet.Backend), + AddressEncoded: sampleBytes, + DataToSign: dataToSign, }, rawHDWallet, hdWallet } diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index afe49888..a17777a6 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -26,7 +26,6 @@ import ( ethwallet "perun.network/go-perun/backend/ethereum/wallet" ethwallettest "perun.network/go-perun/backend/ethereum/wallet/test" pkgtest "perun.network/go-perun/pkg/test" - "perun.network/go-perun/wallet" "perun.network/go-perun/wallet/test" ) @@ -85,15 +84,17 @@ func TestBackend(t *testing.T) { } func newSetup(t require.TestingT) *test.Setup { - acc := ethwallettest.NewTmpWallet().NewAccount() + w := ethwallettest.NewTmpWallet() + acc := w.NewAccount() validAddrBytes, err := hex.DecodeString(validAddr) require.NoError(t, err, "decoding valid address should not fail") return &test.Setup{ - UnlockedAccount: func() (wallet.Account, error) { return acc, nil }, - Backend: new(ethwallet.Backend), - AddressEncoded: validAddrBytes, - DataToSign: dataToSign, + Wallet: w, + Address: acc.Address(), + Backend: new(ethwallet.Backend), + AddressEncoded: validAddrBytes, + DataToSign: dataToSign, } } diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index 9a86977f..2791fe41 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -29,7 +29,6 @@ import ( ethwallet "perun.network/go-perun/backend/ethereum/wallet" "perun.network/go-perun/backend/ethereum/wallet/simple" pkgtest "perun.network/go-perun/pkg/test" - "perun.network/go-perun/wallet" "perun.network/go-perun/wallet/test" ) @@ -59,8 +58,7 @@ func TestUnlock(t *testing.T) { _, err := simpleWallet.Unlock(ethwallet.AsWalletAddr(missingAddr)) assert.Error(t, err, "should error on unlocking missing address") - validAcc, _ := setup.UnlockedAccount() - acc, err := simpleWallet.Unlock(validAcc.Address()) + acc, err := simpleWallet.Unlock(setup.Address) assert.NoError(t, err, "should not error on unlocking missing address") assert.NotNil(t, acc, "account should be non nil when error is nil") } @@ -71,9 +69,7 @@ func TestWallet_Contains(t *testing.T) { missingAddr := common.BytesToAddress(setup.AddressEncoded) assert.False(t, simpleWallet.Contains(missingAddr)) - validAcc, err := setup.UnlockedAccount() - require.NoError(t, err) - assert.True(t, simpleWallet.Contains(ethwallet.AsEthAddr(validAcc.Address()))) + assert.True(t, simpleWallet.Contains(ethwallet.AsEthAddr(setup.Address))) } func TestSignatures(t *testing.T) { @@ -110,9 +106,10 @@ func newSetup(t require.TestingT, prng *rand.Rand) (*test.Setup, *simple.Wallet) require.NoError(t, err, "invalid sample address") return &test.Setup{ - UnlockedAccount: func() (wallet.Account, error) { return acc, nil }, - Backend: new(ethwallet.Backend), - AddressEncoded: validAddrBytes, - DataToSign: dataToSign, + Wallet: simpleWallet, + Address: acc.Address(), + Backend: new(ethwallet.Backend), + AddressEncoded: validAddrBytes, + DataToSign: dataToSign, }, simpleWallet } diff --git a/backend/sim/wallet/wallet_internal_test.go b/backend/sim/wallet/wallet_internal_test.go index 4eeb057b..514420a0 100644 --- a/backend/sim/wallet/wallet_internal_test.go +++ b/backend/sim/wallet/wallet_internal_test.go @@ -22,7 +22,6 @@ import ( "github.com/stretchr/testify/assert" pkgtest "perun.network/go-perun/pkg/test" - "perun.network/go-perun/wallet" "perun.network/go-perun/wallet/test" ) @@ -89,15 +88,22 @@ func TestGenericTests(t *testing.T) { } } -// nolint: interfacer func newWalletSetup(rng *rand.Rand) *test.Setup { - accountA := NewRandomAccount(rng) + w := NewWallet() + acc := w.NewRandomAccount(rng) accountB := NewRandomAccount(rng) - unlockedAccount := func() (wallet.Account, error) { return accountA, nil } + + data := make([]byte, 128) + _, err := rng.Read(data) + if err != nil { + panic(err) + } return &test.Setup{ - Backend: new(Backend), - UnlockedAccount: unlockedAccount, - AddressEncoded: accountB.Address().Bytes(), + Backend: new(Backend), + Wallet: w, + Address: acc.Address(), + AddressEncoded: accountB.Address().Bytes(), + DataToSign: data, } } diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index 362d8446..acbb99e4 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -32,10 +32,11 @@ type UnlockedAccount func() (wallet.Account, error) // Setup provides all objects needed for the generic tests. type Setup struct { - UnlockedAccount UnlockedAccount // provides an account that is ready to sign + Backend wallet.Backend // backend implementation + Wallet wallet.Wallet // the wallet instance used for testing + Address wallet.Address // an address of an account in the test wallet // Address tests - AddressEncoded []byte // a valid nonzero address not in the wallet - Backend wallet.Backend // backend implementation + AddressEncoded []byte // a valid nonzero address not in the wallet // Signature tests DataToSign []byte } @@ -43,55 +44,57 @@ type Setup struct { // GenericSignatureTest runs a test suite designed to test the general functionality of an account. // This function should be called by every implementation of the wallet interface. func GenericSignatureTest(t *testing.T, s *Setup) { - acc, err := s.UnlockedAccount() + acc, err := s.Wallet.Unlock(s.Address) assert.NoError(t, err) // Check unlocked account - sign, err := acc.SignData(s.DataToSign) + sig, err := acc.SignData(s.DataToSign) assert.NoError(t, err, "Sign with unlocked account should succeed") - valid, err := s.Backend.VerifySignature(s.DataToSign, sign, acc.Address()) + valid, err := s.Backend.VerifySignature(s.DataToSign, sig, acc.Address()) assert.True(t, valid, "Verification should succeed") assert.NoError(t, err, "Verification should not produce error") addr, err := s.Backend.DecodeAddress(bytes.NewBuffer(s.AddressEncoded)) assert.NoError(t, err, "Byte deserialization of address should work") - valid, err = s.Backend.VerifySignature(s.DataToSign, sign, addr) + valid, err = s.Backend.VerifySignature(s.DataToSign, sig, addr) assert.False(t, valid, "Verification with wrong address should fail") assert.NoError(t, err, "Verification of valid signature should not produce error") - tampered := make([]byte, len(sign)) - copy(tampered, sign) + tampered := make([]byte, len(sig)) + copy(tampered, sig) // Invalidate the signature and check for error - tampered[0] = ^sign[0] + tampered[0] = ^sig[0] valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) if valid && err == nil { t.Error("Verification of invalid signature should produce error or return false") } // Truncate the signature and check for error - tampered = sign[:len(sign)-1] + tampered = sig[:len(sig)-1] valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) if valid && err != nil { t.Error("Verification of invalid signature should produce error or return false") } // Expand the signature and check for error // nolint:gocritic - tampered = append(sign, 0) + tampered = append(sig, 0) valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) if valid && err != nil { t.Error("Verification of invalid signature should produce error or return false") } // Test DecodeSig - sign, err = acc.SignData(s.DataToSign) + sig, err = acc.SignData(s.DataToSign) require.NoError(t, err, "Sign with unlocked account should succeed") buff := new(bytes.Buffer) - io.Encode(buff, sign) + err = io.Encode(buff, sig) + require.NoError(t, err, "encode sig") sign2, err := s.Backend.DecodeSig(buff) assert.NoError(t, err, "Decoded signature should work") - assert.Equal(t, sign, sign2, "Decoded signature should be equal to the original") + assert.Equal(t, sig, sign2, "Decoded signature should be equal to the original") // Test DecodeSig on short stream - io.Encode(buff, sign) + err = io.Encode(buff, sig) + require.NoError(t, err, "encode sig") shortBuff := bytes.NewBuffer(buff.Bytes()[:len(buff.Bytes())-1]) // remove one byte _, err = s.Backend.DecodeSig(shortBuff) assert.Error(t, err, "DecodeSig on short stream should error") @@ -100,7 +103,7 @@ func GenericSignatureTest(t *testing.T, s *Setup) { // GenericSignatureSizeTest tests that the size of the signatures produced by // Account.Sign(…) does not vary between executions (tested with 2048 samples). func GenericSignatureSizeTest(t *testing.T, s *Setup) { - acc, err := s.UnlockedAccount() + acc, err := s.Wallet.Unlock(s.Address) require.NoError(t, err) // get a signature sign, err := acc.SignData(s.DataToSign) diff --git a/wallet/test/walletbench.go b/wallet/test/walletbench.go index 8c26b402..5b828a1f 100644 --- a/wallet/test/walletbench.go +++ b/wallet/test/walletbench.go @@ -28,7 +28,7 @@ func GenericAccountBenchmark(b *testing.B, s *Setup) { } func benchAccountSign(b *testing.B, s *Setup) { - perunAcc, err := s.UnlockedAccount() + perunAcc, err := s.Wallet.Unlock(s.Address) require.Nil(b, err) for n := 0; n < b.N; n++ { @@ -50,7 +50,7 @@ func GenericBackendBenchmark(b *testing.B, s *Setup) { func benchBackendVerifySig(b *testing.B, s *Setup) { // We dont want to measure the SignDataWithPW here, just need it for the verification b.StopTimer() - perunAcc, err := s.UnlockedAccount() + perunAcc, err := s.Wallet.Unlock(s.Address) require.Nil(b, err) signature, err := perunAcc.SignData(s.DataToSign) require.Nil(b, err) From 1a62db6b3c044259a5968b3241b9852b0d14121b Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 23 Sep 2021 09:51:39 +0200 Subject: [PATCH 6/9] :truck: [wallet/test,backend/.../wallet] Rename GenericSignatureTest -> TestAccountWithWalletAndBackend Naming schema: Include in name which interface types are tested. Signed-off-by: Matthias Geihs --- backend/ethereum/wallet/hd/wallet_test.go | 2 +- backend/ethereum/wallet/keystore/wallet_test.go | 2 +- backend/ethereum/wallet/simple/wallet_test.go | 2 +- backend/sim/wallet/wallet_internal_test.go | 2 +- wallet/test/wallet.go | 17 ++++++++--------- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/backend/ethereum/wallet/hd/wallet_test.go b/backend/ethereum/wallet/hd/wallet_test.go index 4fbba558..805d4a07 100644 --- a/backend/ethereum/wallet/hd/wallet_test.go +++ b/backend/ethereum/wallet/hd/wallet_test.go @@ -36,7 +36,7 @@ var sampleAddr = "1234560000000000000000000000000000000000" func TestGenericSignatureTests(t *testing.T) { s, _, _ := newSetup(t, pkgtest.Prng(t)) - test.GenericSignatureTest(t, s) + test.TestAccountWithWalletAndBackend(t, s) test.GenericSignatureSizeTest(t, s) test.GenericAddressTest(t, s) } diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index a17777a6..84ae9a77 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -38,7 +38,7 @@ const ( func TestGenericSignatureTests(t *testing.T) { setup := newSetup(t) - test.GenericSignatureTest(t, setup) + test.TestAccountWithWalletAndBackend(t, setup) test.GenericSignatureSizeTest(t, setup) } diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index 2791fe41..16bd5079 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -38,7 +38,7 @@ const sampleAddr = "1234560000000000000000000000000000000000" func TestGenericSignatureTests(t *testing.T) { setup, _ := newSetup(t, pkgtest.Prng(t)) - test.GenericSignatureTest(t, setup) + test.TestAccountWithWalletAndBackend(t, setup) test.GenericSignatureSizeTest(t, setup) test.GenericAddressTest(t, setup) } diff --git a/backend/sim/wallet/wallet_internal_test.go b/backend/sim/wallet/wallet_internal_test.go index 514420a0..62ccb537 100644 --- a/backend/sim/wallet/wallet_internal_test.go +++ b/backend/sim/wallet/wallet_internal_test.go @@ -64,7 +64,7 @@ func TestGenericTests(t *testing.T) { t.Run("Generic Signature Test", func(t *testing.T) { t.Parallel() rng := pkgtest.Prng(t, "signature") - test.GenericSignatureTest(t, newWalletSetup(rng)) + test.TestAccountWithWalletAndBackend(t, newWalletSetup(rng)) test.GenericSignatureSizeTest(t, newWalletSetup(rng)) }) diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index acbb99e4..abd7d504 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -32,18 +32,17 @@ type UnlockedAccount func() (wallet.Account, error) // Setup provides all objects needed for the generic tests. type Setup struct { - Backend wallet.Backend // backend implementation - Wallet wallet.Wallet // the wallet instance used for testing - Address wallet.Address // an address of an account in the test wallet - // Address tests - AddressEncoded []byte // a valid nonzero address not in the wallet - // Signature tests - DataToSign []byte + Backend wallet.Backend // backend implementation + Wallet wallet.Wallet // the wallet instance used for testing + Address wallet.Address // an address of an account in the test wallet + DataToSign []byte // some data to sign + AddressEncoded []byte // a valid nonzero address not in the wallet } -// GenericSignatureTest runs a test suite designed to test the general functionality of an account. +// TestAccountWithWalletAndBackend tests an account implementation together with +// a corresponding wallet and backend implementation. // This function should be called by every implementation of the wallet interface. -func GenericSignatureTest(t *testing.T, s *Setup) { +func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { acc, err := s.Wallet.Unlock(s.Address) assert.NoError(t, err) // Check unlocked account From ac59fd943fd93dff0e24ee12d79745dfa68d3371 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 23 Sep 2021 09:54:01 +0200 Subject: [PATCH 7/9] :truck: [wallet/test,backend/.../wallet] Rename GenericAddressTest -> TestAddress Naming schema: Include in name which interface type is tested. Signed-off-by: Matthias Geihs --- backend/ethereum/wallet/hd/wallet_test.go | 2 +- backend/ethereum/wallet/keystore/wallet_test.go | 2 +- backend/ethereum/wallet/simple/wallet_test.go | 2 +- backend/sim/wallet/wallet_internal_test.go | 2 +- wallet/test/address.go | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/backend/ethereum/wallet/hd/wallet_test.go b/backend/ethereum/wallet/hd/wallet_test.go index 805d4a07..3c63a9ed 100644 --- a/backend/ethereum/wallet/hd/wallet_test.go +++ b/backend/ethereum/wallet/hd/wallet_test.go @@ -38,7 +38,7 @@ func TestGenericSignatureTests(t *testing.T) { s, _, _ := newSetup(t, pkgtest.Prng(t)) test.TestAccountWithWalletAndBackend(t, s) test.GenericSignatureSizeTest(t, s) - test.GenericAddressTest(t, s) + test.TestAddress(t, s) } func TestNewWallet(t *testing.T) { diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index 84ae9a77..603a166a 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -43,7 +43,7 @@ func TestGenericSignatureTests(t *testing.T) { } func TestGenericAddressTests(t *testing.T) { - test.GenericAddressTest(t, newSetup(t)) + test.TestAddress(t, newSetup(t)) } func TestWallet_Contains(t *testing.T) { diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index 16bd5079..b3ad95ab 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -40,7 +40,7 @@ func TestGenericSignatureTests(t *testing.T) { setup, _ := newSetup(t, pkgtest.Prng(t)) test.TestAccountWithWalletAndBackend(t, setup) test.GenericSignatureSizeTest(t, setup) - test.GenericAddressTest(t, setup) + test.TestAddress(t, setup) } func TestNewWallet(t *testing.T) { diff --git a/backend/sim/wallet/wallet_internal_test.go b/backend/sim/wallet/wallet_internal_test.go index 62ccb537..35d77bc5 100644 --- a/backend/sim/wallet/wallet_internal_test.go +++ b/backend/sim/wallet/wallet_internal_test.go @@ -59,7 +59,7 @@ func TestGenericTests(t *testing.T) { t.Run("Generic Address Test", func(t *testing.T) { t.Parallel() rng := pkgtest.Prng(t, "address") - test.GenericAddressTest(t, newWalletSetup(rng)) + test.TestAddress(t, newWalletSetup(rng)) }) t.Run("Generic Signature Test", func(t *testing.T) { t.Parallel() diff --git a/wallet/test/address.go b/wallet/test/address.go index ac78885c..e94f6d0a 100644 --- a/wallet/test/address.go +++ b/wallet/test/address.go @@ -22,9 +22,9 @@ import ( "github.com/stretchr/testify/require" ) -// GenericAddressTest runs a test suite designed to test the general functionality of addresses. -// This function should be called by every implementation of the wallet interface. -func GenericAddressTest(t *testing.T, s *Setup) { +// TestAddress runs a test suite designed to test the general functionality of +// an address implementation. +func TestAddress(t *testing.T, s *Setup) { addrLen := len(s.AddressEncoded) null, err := s.Backend.DecodeAddress(bytes.NewReader(make([]byte, addrLen))) assert.NoError(t, err, "Byte deserialization of zero address should work") From 37a07f9ccee79d6fa0b0d312d399fb4dbd0c6e8b Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 23 Sep 2021 15:08:57 +0200 Subject: [PATCH 8/9] :truck: [wallet/test,backend/.../wallet] Rename Address -> AddressInWallet Signed-off-by: Matthias Geihs --- backend/ethereum/wallet/hd/wallet_test.go | 14 +++++++------- backend/ethereum/wallet/keystore/wallet_test.go | 10 +++++----- backend/ethereum/wallet/simple/wallet_test.go | 14 +++++++------- backend/sim/wallet/wallet_internal_test.go | 10 +++++----- wallet/test/wallet.go | 14 +++++++------- wallet/test/walletbench.go | 4 ++-- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/backend/ethereum/wallet/hd/wallet_test.go b/backend/ethereum/wallet/hd/wallet_test.go index 3c63a9ed..5ad8c941 100644 --- a/backend/ethereum/wallet/hd/wallet_test.go +++ b/backend/ethereum/wallet/hd/wallet_test.go @@ -86,7 +86,7 @@ func TestUnlock(t *testing.T) { _, err := hdWallet.Unlock(ethwallet.AsWalletAddr(missingAddr)) assert.Error(t, err, "should error on unlocking missing address") - acc, err := hdWallet.Unlock(setup.Address) + acc, err := hdWallet.Unlock(setup.AddressInWallet) assert.NoError(t, err, "should not error on unlocking valid address") assert.NotNil(t, acc, "account should be non nil when error is nil") } @@ -99,7 +99,7 @@ func TestContains(t *testing.T) { missingAddr := common.BytesToAddress(setup.AddressEncoded) assert.False(t, hdWallet.Contains(missingAddr), "should not contain address of the missing account") - assert.True(t, hdWallet.Contains(ethwallet.AsEthAddr(setup.Address)), "should contain valid account") + assert.True(t, hdWallet.Contains(ethwallet.AsEthAddr(setup.AddressInWallet)), "should contain valid account") } // nolint:interfacer // rand.Rand is preferred over io.Reader here. @@ -125,10 +125,10 @@ func newSetup(t require.TestingT, prng *rand.Rand) (*test.Setup, accounts.Wallet require.NoError(t, err, "invalid sample address") return &test.Setup{ - Wallet: hdWallet, - Address: acc.Address(), - Backend: new(ethwallet.Backend), - AddressEncoded: sampleBytes, - DataToSign: dataToSign, + Wallet: hdWallet, + AddressInWallet: acc.Address(), + Backend: new(ethwallet.Backend), + AddressEncoded: sampleBytes, + DataToSign: dataToSign, }, rawHDWallet, hdWallet } diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index 603a166a..66d71f3a 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -90,11 +90,11 @@ func newSetup(t require.TestingT) *test.Setup { require.NoError(t, err, "decoding valid address should not fail") return &test.Setup{ - Wallet: w, - Address: acc.Address(), - Backend: new(ethwallet.Backend), - AddressEncoded: validAddrBytes, - DataToSign: dataToSign, + Wallet: w, + AddressInWallet: acc.Address(), + Backend: new(ethwallet.Backend), + AddressEncoded: validAddrBytes, + DataToSign: dataToSign, } } diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index b3ad95ab..dd51e29f 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -58,7 +58,7 @@ func TestUnlock(t *testing.T) { _, err := simpleWallet.Unlock(ethwallet.AsWalletAddr(missingAddr)) assert.Error(t, err, "should error on unlocking missing address") - acc, err := simpleWallet.Unlock(setup.Address) + acc, err := simpleWallet.Unlock(setup.AddressInWallet) assert.NoError(t, err, "should not error on unlocking missing address") assert.NotNil(t, acc, "account should be non nil when error is nil") } @@ -69,7 +69,7 @@ func TestWallet_Contains(t *testing.T) { missingAddr := common.BytesToAddress(setup.AddressEncoded) assert.False(t, simpleWallet.Contains(missingAddr)) - assert.True(t, simpleWallet.Contains(ethwallet.AsEthAddr(setup.Address))) + assert.True(t, simpleWallet.Contains(ethwallet.AsEthAddr(setup.AddressInWallet))) } func TestSignatures(t *testing.T) { @@ -106,10 +106,10 @@ func newSetup(t require.TestingT, prng *rand.Rand) (*test.Setup, *simple.Wallet) require.NoError(t, err, "invalid sample address") return &test.Setup{ - Wallet: simpleWallet, - Address: acc.Address(), - Backend: new(ethwallet.Backend), - AddressEncoded: validAddrBytes, - DataToSign: dataToSign, + Wallet: simpleWallet, + AddressInWallet: acc.Address(), + Backend: new(ethwallet.Backend), + AddressEncoded: validAddrBytes, + DataToSign: dataToSign, }, simpleWallet } diff --git a/backend/sim/wallet/wallet_internal_test.go b/backend/sim/wallet/wallet_internal_test.go index 35d77bc5..3a4a346b 100644 --- a/backend/sim/wallet/wallet_internal_test.go +++ b/backend/sim/wallet/wallet_internal_test.go @@ -100,10 +100,10 @@ func newWalletSetup(rng *rand.Rand) *test.Setup { } return &test.Setup{ - Backend: new(Backend), - Wallet: w, - Address: acc.Address(), - AddressEncoded: accountB.Address().Bytes(), - DataToSign: data, + Backend: new(Backend), + Wallet: w, + AddressInWallet: acc.Address(), + AddressEncoded: accountB.Address().Bytes(), + DataToSign: data, } } diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index abd7d504..82d7f33a 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -32,18 +32,18 @@ type UnlockedAccount func() (wallet.Account, error) // Setup provides all objects needed for the generic tests. type Setup struct { - Backend wallet.Backend // backend implementation - Wallet wallet.Wallet // the wallet instance used for testing - Address wallet.Address // an address of an account in the test wallet - DataToSign []byte // some data to sign - AddressEncoded []byte // a valid nonzero address not in the wallet + Backend wallet.Backend // backend implementation + Wallet wallet.Wallet // the wallet instance used for testing + AddressInWallet wallet.Address // an address of an account in the test wallet + DataToSign []byte // some data to sign + AddressEncoded []byte // a valid nonzero address not in the wallet } // TestAccountWithWalletAndBackend tests an account implementation together with // a corresponding wallet and backend implementation. // This function should be called by every implementation of the wallet interface. func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { - acc, err := s.Wallet.Unlock(s.Address) + acc, err := s.Wallet.Unlock(s.AddressInWallet) assert.NoError(t, err) // Check unlocked account sig, err := acc.SignData(s.DataToSign) @@ -102,7 +102,7 @@ func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { // GenericSignatureSizeTest tests that the size of the signatures produced by // Account.Sign(…) does not vary between executions (tested with 2048 samples). func GenericSignatureSizeTest(t *testing.T, s *Setup) { - acc, err := s.Wallet.Unlock(s.Address) + acc, err := s.Wallet.Unlock(s.AddressInWallet) require.NoError(t, err) // get a signature sign, err := acc.SignData(s.DataToSign) diff --git a/wallet/test/walletbench.go b/wallet/test/walletbench.go index 5b828a1f..9e9d01ea 100644 --- a/wallet/test/walletbench.go +++ b/wallet/test/walletbench.go @@ -28,7 +28,7 @@ func GenericAccountBenchmark(b *testing.B, s *Setup) { } func benchAccountSign(b *testing.B, s *Setup) { - perunAcc, err := s.Wallet.Unlock(s.Address) + perunAcc, err := s.Wallet.Unlock(s.AddressInWallet) require.Nil(b, err) for n := 0; n < b.N; n++ { @@ -50,7 +50,7 @@ func GenericBackendBenchmark(b *testing.B, s *Setup) { func benchBackendVerifySig(b *testing.B, s *Setup) { // We dont want to measure the SignDataWithPW here, just need it for the verification b.StopTimer() - perunAcc, err := s.Wallet.Unlock(s.Address) + perunAcc, err := s.Wallet.Unlock(s.AddressInWallet) require.Nil(b, err) signature, err := perunAcc.SignData(s.DataToSign) require.Nil(b, err) From 2d44fd3302a52bd18a175b8bd12b64e97c7acb1f Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Tue, 5 Oct 2021 11:58:51 +0200 Subject: [PATCH 9/9] :adhesive_bandage: [wallet/test,backend/.../wallet] Test setup: Explicit zero address Before, the wallet address test was generating a zero address assuming a specific address encoding. The assumption was that address encodings always have the same length and that decoding a zero bytes array of that length gives the zero address. However, while this was true for the existing backends, it may not be true for all future backends. Hence, we now provide the zero address specifically with the test setup. Signed-off-by: Matthias Geihs --- backend/ethereum/wallet/hd/wallet_test.go | 1 + backend/ethereum/wallet/keystore/wallet_test.go | 1 + backend/ethereum/wallet/simple/wallet_test.go | 1 + backend/sim/wallet/wallet_internal_test.go | 14 ++++++++++++-- wallet/test/address.go | 9 ++++++--- wallet/test/wallet.go | 1 + 6 files changed, 22 insertions(+), 5 deletions(-) diff --git a/backend/ethereum/wallet/hd/wallet_test.go b/backend/ethereum/wallet/hd/wallet_test.go index 5ad8c941..4280a450 100644 --- a/backend/ethereum/wallet/hd/wallet_test.go +++ b/backend/ethereum/wallet/hd/wallet_test.go @@ -129,6 +129,7 @@ func newSetup(t require.TestingT, prng *rand.Rand) (*test.Setup, accounts.Wallet AddressInWallet: acc.Address(), Backend: new(ethwallet.Backend), AddressEncoded: sampleBytes, + ZeroAddress: ethwallet.AsWalletAddr(common.Address{}), DataToSign: dataToSign, }, rawHDWallet, hdWallet } diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index 66d71f3a..4e72bf8f 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -94,6 +94,7 @@ func newSetup(t require.TestingT) *test.Setup { AddressInWallet: acc.Address(), Backend: new(ethwallet.Backend), AddressEncoded: validAddrBytes, + ZeroAddress: ethwallet.AsWalletAddr(common.Address{}), DataToSign: dataToSign, } } diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index dd51e29f..9d29a4c1 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -110,6 +110,7 @@ func newSetup(t require.TestingT, prng *rand.Rand) (*test.Setup, *simple.Wallet) AddressInWallet: acc.Address(), Backend: new(ethwallet.Backend), AddressEncoded: validAddrBytes, + ZeroAddress: ethwallet.AsWalletAddr(common.Address{}), DataToSign: dataToSign, }, simpleWallet } diff --git a/backend/sim/wallet/wallet_internal_test.go b/backend/sim/wallet/wallet_internal_test.go index 3a4a346b..d0b8f713 100644 --- a/backend/sim/wallet/wallet_internal_test.go +++ b/backend/sim/wallet/wallet_internal_test.go @@ -15,6 +15,7 @@ package wallet import ( + "bytes" "math/big" "math/rand" "testing" @@ -99,11 +100,20 @@ func newWalletSetup(rng *rand.Rand) *test.Setup { panic(err) } + backend := new(Backend) + addrEncoded := accountB.Address().Bytes() + addrLen := len(addrEncoded) + zeroAddr, err := backend.DecodeAddress(bytes.NewReader(make([]byte, addrLen))) + if err != nil { + panic(err) + } + return &test.Setup{ - Backend: new(Backend), + Backend: backend, Wallet: w, AddressInWallet: acc.Address(), - AddressEncoded: accountB.Address().Bytes(), + AddressEncoded: addrEncoded, + ZeroAddress: zeroAddr, DataToSign: data, } } diff --git a/wallet/test/address.go b/wallet/test/address.go index e94f6d0a..6fefe6a8 100644 --- a/wallet/test/address.go +++ b/wallet/test/address.go @@ -25,9 +25,7 @@ import ( // TestAddress runs a test suite designed to test the general functionality of // an address implementation. func TestAddress(t *testing.T, s *Setup) { - addrLen := len(s.AddressEncoded) - null, err := s.Backend.DecodeAddress(bytes.NewReader(make([]byte, addrLen))) - assert.NoError(t, err, "Byte deserialization of zero address should work") + null := s.ZeroAddress addr, err := s.Backend.DecodeAddress(bytes.NewReader(s.AddressEncoded)) assert.NoError(t, err, "Byte deserialization of address should work") @@ -42,6 +40,11 @@ func TestAddress(t *testing.T, s *Setup) { assert.False(t, addr.Equals(null), "Expected inequality of zero, nonzero address") assert.True(t, null.Equals(null), "Expected equality of zero address to itself") + // Test Address.Cmp. + assert.Positive(t, addr.Cmp(null), "Expected addr > zero") + assert.Zero(t, null.Cmp(null), "Expected zero = zero") + assert.Negative(t, null.Cmp(addr), "Expected null < addr") + // Test Address.Bytes. addrBytes := addr.Bytes() nullBytes := null.Bytes() diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index 82d7f33a..14de689b 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -35,6 +35,7 @@ type Setup struct { Backend wallet.Backend // backend implementation Wallet wallet.Wallet // the wallet instance used for testing AddressInWallet wallet.Address // an address of an account in the test wallet + ZeroAddress wallet.Address // an address that is less or equal to any other address DataToSign []byte // some data to sign AddressEncoded []byte // a valid nonzero address not in the wallet }