From 009057744642b6a19886bfdc01155b1319297331 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 15:27:38 +0300 Subject: [PATCH 01/15] wallet: don't permanently store wif in the Account It's useless and it's just another copy of the key. If really needed, it can be derived from the key. --- pkg/wallet/account.go | 6 ------ pkg/wallet/account_test.go | 2 +- pkg/wallet/wallet_test.go | 2 -- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 4abc659f1..067d59e07 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -24,9 +24,6 @@ type Account struct { // NEO public key. publicKey []byte - // Account import file. - wif string - // NEO public address. Address string `json:"address"` @@ -164,7 +161,6 @@ func (a *Account) Decrypt(passphrase string, scrypt keys.ScryptParams) error { } a.publicKey = a.privateKey.PublicKey().Bytes() - a.wif = a.privateKey.WIF() return nil } @@ -239,13 +235,11 @@ func (a *Account) ConvertMultisig(m int, pubs []*keys.PublicKey) error { func NewAccountFromPrivateKey(p *keys.PrivateKey) *Account { pubKey := p.PublicKey() pubAddr := p.Address() - wif := p.WIF() a := &Account{ publicKey: pubKey.Bytes(), privateKey: p, Address: pubAddr, - wif: wif, Contract: &Contract{ Script: pubKey.GetVerificationScript(), Parameters: getContractParams(1), diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 2d0a798df..d2b0374de 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -215,7 +215,7 @@ func convertPubs(t *testing.T, hexKeys []string) []*keys.PublicKey { func compareFields(t *testing.T, tk keytestcases.Ktype, acc *Account) { want, have := tk.Address, acc.Address require.Equalf(t, want, have, "expected address %s got %s", want, have) - want, have = tk.Wif, acc.wif + want, have = tk.Wif, acc.privateKey.WIF() require.Equalf(t, want, have, "expected wif %s got %s", want, have) want, have = tk.PublicKey, hex.EncodeToString(acc.publicKey) require.Equalf(t, want, have, "expected pub key %s got %s", want, have) diff --git a/pkg/wallet/wallet_test.go b/pkg/wallet/wallet_test.go index 6ac8f4071..42d078663 100644 --- a/pkg/wallet/wallet_test.go +++ b/pkg/wallet/wallet_test.go @@ -49,7 +49,6 @@ func TestAddAccount(t *testing.T) { wallet.AddAccount(&Account{ privateKey: nil, publicKey: nil, - wif: "", Address: "real", EncryptedWIF: "", Label: "", @@ -79,7 +78,6 @@ func TestSave(t *testing.T) { wallet.AddAccount(&Account{ privateKey: nil, publicKey: nil, - wif: "", Address: "", EncryptedWIF: "", Label: "", From 53edbd569fb8d410b546687d72966609f90b5cad Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 16:39:48 +0300 Subject: [PATCH 02/15] wallet: don't allow to ConvertMultisig a locked account This stretched the definition of Locked somewhat, but still makes sense to me, locked accounts better not be touched. --- pkg/wallet/account.go | 3 +++ pkg/wallet/account_test.go | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 067d59e07..c3a0ba098 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -205,6 +205,9 @@ func NewAccountFromEncryptedWIF(wif string, pass string, scrypt keys.ScryptParam // ConvertMultisig sets a's contract to multisig contract with m sufficient signatures. func (a *Account) ConvertMultisig(m int, pubs []*keys.PublicKey) error { + if a.Locked { + return errors.New("account is locked") + } var found bool for i := range pubs { if bytes.Equal(a.publicKey, pubs[i].Bytes()) { diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index d2b0374de..841201edb 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -179,6 +179,12 @@ func TestAccount_ConvertMultisig(t *testing.T) { "03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699", } + t.Run("locked", func(t *testing.T) { + a.Locked = true + pubs := convertPubs(t, hexs) + require.Error(t, a.ConvertMultisig(1, pubs)) + a.Locked = false + }) t.Run("invalid number of signatures", func(t *testing.T) { pubs := convertPubs(t, hexs) require.Error(t, a.ConvertMultisig(0, pubs)) From a30e73a0d73be8a50ee95de2c8e0e91269afc053 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 16:44:50 +0300 Subject: [PATCH 03/15] wallet: drop publicKey from Account It's not very useful and it's only available when we have a private key anyway. --- pkg/wallet/account.go | 13 +++++-------- pkg/wallet/account_test.go | 9 ++++++++- pkg/wallet/wallet_test.go | 2 -- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index c3a0ba098..ef426c08e 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -1,7 +1,6 @@ package wallet import ( - "bytes" "errors" "fmt" @@ -21,9 +20,6 @@ type Account struct { // NEO private key. privateKey *keys.PrivateKey - // NEO public key. - publicKey []byte - // NEO public address. Address string `json:"address"` @@ -160,8 +156,6 @@ func (a *Account) Decrypt(passphrase string, scrypt keys.ScryptParams) error { return err } - a.publicKey = a.privateKey.PublicKey().Bytes() - return nil } @@ -208,9 +202,13 @@ func (a *Account) ConvertMultisig(m int, pubs []*keys.PublicKey) error { if a.Locked { return errors.New("account is locked") } + if a.privateKey == nil { + return errors.New("account key is not available (need to decrypt?)") + } var found bool + accKey := a.privateKey.PublicKey() for i := range pubs { - if bytes.Equal(a.publicKey, pubs[i].Bytes()) { + if accKey.Equal(pubs[i]) { found = true break } @@ -240,7 +238,6 @@ func NewAccountFromPrivateKey(p *keys.PrivateKey) *Account { pubAddr := p.Address() a := &Account{ - publicKey: pubKey.Bytes(), privateKey: p, Address: pubAddr, Contract: &Contract{ diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 841201edb..f424f779c 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -185,6 +185,13 @@ func TestAccount_ConvertMultisig(t *testing.T) { require.Error(t, a.ConvertMultisig(1, pubs)) a.Locked = false }) + t.Run("no private key", func(t *testing.T) { + pk := a.privateKey + a.privateKey = nil + pubs := convertPubs(t, hexs) + require.Error(t, a.ConvertMultisig(0, pubs)) + a.privateKey = pk + }) t.Run("invalid number of signatures", func(t *testing.T) { pubs := convertPubs(t, hexs) require.Error(t, a.ConvertMultisig(0, pubs)) @@ -223,7 +230,7 @@ func compareFields(t *testing.T, tk keytestcases.Ktype, acc *Account) { require.Equalf(t, want, have, "expected address %s got %s", want, have) want, have = tk.Wif, acc.privateKey.WIF() require.Equalf(t, want, have, "expected wif %s got %s", want, have) - want, have = tk.PublicKey, hex.EncodeToString(acc.publicKey) + want, have = tk.PublicKey, hex.EncodeToString(acc.privateKey.PublicKey().Bytes()) require.Equalf(t, want, have, "expected pub key %s got %s", want, have) want, have = tk.PrivateKey, acc.privateKey.String() require.Equalf(t, want, have, "expected priv key %s got %s", want, have) diff --git a/pkg/wallet/wallet_test.go b/pkg/wallet/wallet_test.go index 42d078663..98f940cd6 100644 --- a/pkg/wallet/wallet_test.go +++ b/pkg/wallet/wallet_test.go @@ -48,7 +48,6 @@ func TestAddAccount(t *testing.T) { wallet.AddAccount(&Account{ privateKey: nil, - publicKey: nil, Address: "real", EncryptedWIF: "", Label: "", @@ -77,7 +76,6 @@ func TestSave(t *testing.T) { wallet.AddAccount(&Account{ privateKey: nil, - publicKey: nil, Address: "", EncryptedWIF: "", Label: "", From 62be6f959c789ffaf53f851fc029358525faa0d2 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 15:28:35 +0300 Subject: [PATCH 04/15] keys/wallet: add Destroy/Close/Close PrivateKey can be destroyed and Account/Wallet can be closed (destroying keys in the process). --- pkg/crypto/keys/private_key.go | 9 +++++++++ pkg/crypto/keys/private_key_test.go | 4 ++++ pkg/wallet/account.go | 11 +++++++++++ pkg/wallet/account_test.go | 4 +++- pkg/wallet/wallet.go | 11 ++++++++--- pkg/wallet/wallet_test.go | 5 ++++- 6 files changed, 39 insertions(+), 5 deletions(-) diff --git a/pkg/crypto/keys/private_key.go b/pkg/crypto/keys/private_key.go index 5e5992378..775c8e556 100644 --- a/pkg/crypto/keys/private_key.go +++ b/pkg/crypto/keys/private_key.go @@ -117,6 +117,15 @@ func (p *PrivateKey) WIF() string { return w } +// Destroy wipes the contents of the private key from memory. Any operations +// with the key after call to Destroy have undefined behavior. +func (p *PrivateKey) Destroy() { + bits := p.D.Bits() + for i := range bits { + bits[i] = 0 + } +} + // Address derives the public NEO address that is coupled with the private key, and // returns it as a string. func (p *PrivateKey) Address() string { diff --git a/pkg/crypto/keys/private_key_test.go b/pkg/crypto/keys/private_key_test.go index b8b806080..16dc84c7a 100644 --- a/pkg/crypto/keys/private_key_test.go +++ b/pkg/crypto/keys/private_key_test.go @@ -2,6 +2,7 @@ package keys import ( "encoding/hex" + "math/big" "strings" "testing" @@ -27,6 +28,9 @@ func TestPrivateKey(t *testing.T) { assert.Equal(t, testCase.Wif, wif) pubKey := privKey.PublicKey() assert.Equal(t, hex.EncodeToString(pubKey.Bytes()), testCase.PublicKey) + oldD := new(big.Int).Set(privKey.D) + privKey.Destroy() + assert.NotEqual(t, oldD, privKey.D) } } diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index ef426c08e..c0bc5910a 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -175,6 +175,17 @@ func (a *Account) PrivateKey() *keys.PrivateKey { return a.privateKey } +// Close cleans up the private key used by Account and disassociates it from +// Account. The Account can no longer sign anything after this call, but Decrypt +// can make it usable again. +func (a *Account) Close() { + if a.privateKey == nil { + return + } + a.privateKey.Destroy() + a.privateKey = nil +} + // NewAccountFromWIF creates a new Account from the given WIF. func NewAccountFromWIF(wif string) (*Account, error) { privKey, err := keys.NewPrivateKeyFromWIF(wif) diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index f424f779c..238430217 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -141,9 +141,11 @@ func TestContractSignTx(t *testing.T) { require.Error(t, acc2.SignTx(0, tx)) // Locked account. acc2.Locked = false - acc2.privateKey = nil + acc2.Close() require.False(t, acc2.CanSign()) require.Error(t, acc2.SignTx(0, tx)) // No private key. + acc2.Close() // No-op. + require.False(t, acc2.CanSign()) tx.Scripts = append(tx.Scripts, transaction.Witness{ VerificationScript: acc.Contract.Script, diff --git a/pkg/wallet/wallet.go b/pkg/wallet/wallet.go index 024a5679b..a5fa85f96 100644 --- a/pkg/wallet/wallet.go +++ b/pkg/wallet/wallet.go @@ -168,9 +168,14 @@ func (w *Wallet) JSON() ([]byte, error) { return json.MarshalIndent(w, " ", " ") } -// Deprecated: Close is deprecated. -// Close performs nothing and is left for backwards compatibility. -func (w *Wallet) Close() {} +// Close closes all Wallet accounts making them incapable of signing anything +// (unless they're decrypted again). It's not doing anything to the underlying +// wallet file. +func (w *Wallet) Close() { + for _, acc := range w.Accounts { + acc.Close() + } +} // GetAccount returns an account corresponding to the provided scripthash. func (w *Wallet) GetAccount(h util.Uint160) *Account { diff --git a/pkg/wallet/wallet_test.go b/pkg/wallet/wallet_test.go index 98f940cd6..7d155e9c4 100644 --- a/pkg/wallet/wallet_test.go +++ b/pkg/wallet/wallet_test.go @@ -34,13 +34,16 @@ func TestNewWalletFromFile_Negative_NoFile(t *testing.T) { require.Errorf(t, err, "open testWallet: no such file or directory") } -func TestCreateAccount(t *testing.T) { +func TestCreateAccountAndClose(t *testing.T) { wallet := checkWalletConstructor(t) errAcc := wallet.CreateAccount("testName", "testPass") require.NoError(t, errAcc) accounts := wallet.Accounts require.Len(t, accounts, 1) + require.True(t, wallet.Accounts[0].CanSign()) + wallet.Close() + require.False(t, wallet.Accounts[0].CanSign()) } func TestAddAccount(t *testing.T) { From 8d33206bb846a24a7edee77e4d748f863caa3440 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 17:52:44 +0300 Subject: [PATCH 05/15] *: don't get private key from account if just public one is needed Add PublicKey() API to the Account and use it as appropriate, avoid creating additional references to the private key. --- cli/contract_test.go | 2 +- cli/smartcontract/manifest.go | 2 +- cli/wallet/multisig.go | 2 +- cli/wallet/validator.go | 4 ++-- internal/basicchain/basic.go | 4 ++-- internal/contracts/contracts_test.go | 2 +- pkg/consensus/consensus.go | 4 +--- pkg/consensus/consensus_test.go | 2 +- pkg/core/blockchain_neotest_test.go | 6 ++--- pkg/core/native/native_test/neo_test.go | 26 +++++++++++----------- pkg/core/native/native_test/oracle_test.go | 4 ++-- pkg/neotest/chain/chain.go | 14 ++++++------ pkg/neotest/signer.go | 4 ++-- pkg/neotest/signer_test.go | 6 ++--- pkg/services/notary/core_test.go | 18 +++++++-------- pkg/services/notary/node.go | 2 +- pkg/services/notary/node_test.go | 8 +++---- pkg/services/oracle/oracle_test.go | 14 ++++++------ pkg/services/stateroot/service_test.go | 8 +++---- pkg/services/stateroot/validators.go | 2 +- pkg/wallet/account.go | 11 ++++++++- pkg/wallet/account_test.go | 5 +++-- 22 files changed, 79 insertions(+), 71 deletions(-) diff --git a/cli/contract_test.go b/cli/contract_test.go index fcea7c4ac..d0cc32596 100644 --- a/cli/contract_test.go +++ b/cli/contract_test.go @@ -802,7 +802,7 @@ func TestComlileAndInvokeFunction(t *testing.T) { require.NoError(t, err) pk, err := keys.NewPrivateKey() require.NoError(t, err) - err = acc.ConvertMultisig(2, keys.PublicKeys{acc.PrivateKey().PublicKey(), pk.PublicKey()}) + err = acc.ConvertMultisig(2, keys.PublicKeys{acc.PublicKey(), pk.PublicKey()}) require.NoError(t, err) t.Run("cosigner is multisig account", func(t *testing.T) { diff --git a/cli/smartcontract/manifest.go b/cli/smartcontract/manifest.go index 498e66f77..a20325208 100644 --- a/cli/smartcontract/manifest.go +++ b/cli/smartcontract/manifest.go @@ -45,7 +45,7 @@ func manifestAddGroup(ctx *cli.Context) error { var found bool sig := gAcc.PrivateKey().Sign(h.BytesBE()) - pub := gAcc.PrivateKey().PublicKey() + pub := gAcc.PublicKey() for i := range m.Groups { if m.Groups[i].PublicKey.Equal(pub) { m.Groups[i].Signature = sig diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index 114dcdf2f..e489a42ef 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -60,7 +60,7 @@ func signStoredTransaction(ctx *cli.Context) error { if acc.CanSign() { priv := acc.PrivateKey() sign := priv.SignHashable(uint32(pc.Network), pc.Verifiable) - if err := pc.AddSignature(ch, acc.Contract, priv.PublicKey(), sign); err != nil { + if err := pc.AddSignature(ch, acc.Contract, acc.PublicKey(), sign); err != nil { return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) } } else if rpcNode == "" { diff --git a/cli/wallet/validator.go b/cli/wallet/validator.go index 09717b46b..ef4726e27 100644 --- a/cli/wallet/validator.go +++ b/cli/wallet/validator.go @@ -76,13 +76,13 @@ func newValidatorCommands() []cli.Command { func handleRegister(ctx *cli.Context) error { return handleNeoAction(ctx, func(contract *neo.Contract, _ util.Uint160, acc *wallet.Account) (*transaction.Transaction, error) { - return contract.RegisterCandidateUnsigned(acc.PrivateKey().PublicKey()) + return contract.RegisterCandidateUnsigned(acc.PublicKey()) }) } func handleUnregister(ctx *cli.Context) error { return handleNeoAction(ctx, func(contract *neo.Contract, _ util.Uint160, acc *wallet.Account) (*transaction.Transaction, error) { - return contract.UnregisterCandidateUnsigned(acc.PrivateKey().PublicKey()) + return contract.UnregisterCandidateUnsigned(acc.PublicKey()) }) } diff --git a/internal/basicchain/basic.go b/internal/basicchain/basic.go index f0071cfd3..304ff53c8 100644 --- a/internal/basicchain/basic.go +++ b/internal/basicchain/basic.go @@ -144,8 +144,8 @@ func Init(t *testing.T, rootpath string, e *neotest.Executor) { require.NoError(t, err) require.NoError(t, ntr.Accounts[0].Decrypt("one", ntr.Scrypt)) designateSuperInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", - int64(noderoles.P2PNotary), []interface{}{ntr.Accounts[0].PrivateKey().PublicKey().Bytes()}) - t.Logf("Designated Notary node: %s", hex.EncodeToString(ntr.Accounts[0].PrivateKey().PublicKey().Bytes())) + int64(noderoles.P2PNotary), []interface{}{ntr.Accounts[0].PublicKey().Bytes()}) + t.Logf("Designated Notary node: %s", hex.EncodeToString(ntr.Accounts[0].PublicKey().Bytes())) // Block #10: push verification contract with arguments into the chain. verifyPath = filepath.Join(testDataPrefix, "verify_args", "verification_with_args_contract.go") diff --git a/internal/contracts/contracts_test.go b/internal/contracts/contracts_test.go index e9e027b36..743bbb234 100644 --- a/internal/contracts/contracts_test.go +++ b/internal/contracts/contracts_test.go @@ -74,7 +74,7 @@ func generateManagementHelperContracts(t *testing.T, saveState bool) { stdHash := e.NativeHash(t, nativenames.StdLib) neoHash := e.NativeHash(t, nativenames.Neo) singleChainValidatorAcc := e.Validator.(neotest.MultiSigner).Single(2).Account() // priv0 - require.NoError(t, singleChainValidatorAcc.ConvertMultisig(1, keys.PublicKeys{singleChainValidatorAcc.PrivateKey().PublicKey()})) + require.NoError(t, singleChainValidatorAcc.ConvertMultisig(1, keys.PublicKeys{singleChainValidatorAcc.PublicKey()})) singleChainValidatorHash := singleChainValidatorAcc.Contract.ScriptHash() w := io.NewBufBinWriter() diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index f4cec74fb..38326225a 100644 --- a/pkg/consensus/consensus.go +++ b/pkg/consensus/consensus.go @@ -377,17 +377,15 @@ func (s *service) getKeyPair(pubs []crypto.PublicKey) (int, crypto.PrivateKey, c continue } - key := acc.PrivateKey() if acc.PrivateKey() == nil { err := acc.Decrypt(s.Config.Wallet.Password, s.wallet.Scrypt) if err != nil { s.log.Fatal("can't unlock account", zap.String("address", address.Uint160ToString(sh))) break } - key = acc.PrivateKey() } - return i, &privateKey{PrivateKey: key}, &publicKey{PublicKey: key.PublicKey()} + return i, &privateKey{PrivateKey: acc.PrivateKey()}, &publicKey{PublicKey: acc.PublicKey()} } return -1, nil, nil diff --git a/pkg/consensus/consensus_test.go b/pkg/consensus/consensus_test.go index 754bb74b0..92aa0a7cc 100644 --- a/pkg/consensus/consensus_test.go +++ b/pkg/consensus/consensus_test.go @@ -115,7 +115,7 @@ func initServiceNextConsensus(t *testing.T, newAcc *wallet.Account, offset uint3 func TestService_NextConsensus(t *testing.T) { newAcc, err := wallet.NewAccount() require.NoError(t, err) - script, err := smartcontract.CreateMajorityMultiSigRedeemScript(keys.PublicKeys{newAcc.PrivateKey().PublicKey()}) + script, err := smartcontract.CreateMajorityMultiSigRedeemScript(keys.PublicKeys{newAcc.PublicKey()}) require.NoError(t, err) checkNextConsensus := func(t *testing.T, bc *core.Blockchain, height uint32, h util.Uint160) { diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index 35d5bc3a1..ffe9f23ba 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -1060,7 +1060,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { notaryServiceFeePerKey := bc.GetNotaryServiceFeePerKey() oracleAcc := accs[2] - oraclePubs := keys.PublicKeys{oracleAcc.PrivateKey().PublicKey()} + oraclePubs := keys.PublicKeys{oracleAcc.PublicKey()} require.NoError(t, oracleAcc.ConvertMultisig(1, oraclePubs)) neoHash := e.NativeHash(t, nativenames.Neo) @@ -1179,7 +1179,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { }) t.Run("CalculateNetworkFee, multisignature script", func(t *testing.T) { multisigAcc := accs[4] - pKeys := keys.PublicKeys{multisigAcc.PrivateKey().PublicKey()} + pKeys := keys.PublicKeys{multisigAcc.PublicKey()} require.NoError(t, multisigAcc.ConvertMultisig(1, pKeys)) multisigHash := hash.Hash160(multisigAcc.Contract.Script) tx := newTestTx(t, multisigHash, testScript) @@ -1594,7 +1594,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { notary, err := wallet.NewAccount() require.NoError(t, err) designateSuperInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", - int64(noderoles.P2PNotary), []interface{}{notary.PrivateKey().PublicKey().Bytes()}) + int64(noderoles.P2PNotary), []interface{}{notary.PublicKey().Bytes()}) txSetNotary := transaction.New([]byte{byte(opcode.RET)}, 0) txSetNotary.Signers = []transaction.Signer{ { diff --git a/pkg/core/native/native_test/neo_test.go b/pkg/core/native/native_test/neo_test.go index 6d8ee3275..ef9e62433 100644 --- a/pkg/core/native/native_test/neo_test.go +++ b/pkg/core/native/native_test/neo_test.go @@ -65,7 +65,7 @@ func TestNEO_CandidateEvents(t *testing.T) { singleSigner := c.Signers[0].(neotest.MultiSigner).Single(0) cc := c.WithSigners(c.Signers[0], singleSigner) e := c.Executor - pkb := singleSigner.Account().PrivateKey().PublicKey().Bytes() + pkb := singleSigner.Account().PublicKey().Bytes() // Register 1 -> event tx := cc.Invoke(t, true, "registerCandidate", pkb) @@ -160,13 +160,13 @@ func TestNEO_Vote(t *testing.T) { transferTx = neoValidatorsInvoker.PrepareInvoke(t, "transfer", e.Validator.ScriptHash(), referenceAccounts[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), int64(committeeSize+1-i)*1000000, nil) txes = append(txes, transferTx) if i > 0 { - registerTx := neoValidatorsInvoker.WithSigners(candidates[i]).PrepareInvoke(t, "registerCandidate", candidates[i].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + registerTx := neoValidatorsInvoker.WithSigners(candidates[i]).PrepareInvoke(t, "registerCandidate", candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes()) txes = append(txes, registerTx) - voteTx := neoValidatorsInvoker.WithSigners(voters[i]).PrepareInvoke(t, "vote", voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[i].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + voteTx := neoValidatorsInvoker.WithSigners(voters[i]).PrepareInvoke(t, "vote", voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes()) txes = append(txes, voteTx) } } - txes = append(txes, policyInvoker.PrepareInvoke(t, "blockAccount", candidates[len(candidates)-1].(neotest.SingleSigner).Account().PrivateKey().PublicKey().GetScriptHash())) + txes = append(txes, policyInvoker.PrepareInvoke(t, "blockAccount", candidates[len(candidates)-1].(neotest.SingleSigner).Account().PublicKey().GetScriptHash())) neoValidatorsInvoker.AddNewBlock(t, txes...) for _, tx := range txes { e.CheckHalt(t, tx.Hash(), stackitem.Make(true)) // luckily, both `transfer`, `registerCandidate` and `vote` return boolean values @@ -184,9 +184,9 @@ func TestNEO_Vote(t *testing.T) { // Register and give some value to the last validator. txes = txes[:0] - registerTx := neoValidatorsInvoker.WithSigners(candidates[0]).PrepareInvoke(t, "registerCandidate", candidates[0].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + registerTx := neoValidatorsInvoker.WithSigners(candidates[0]).PrepareInvoke(t, "registerCandidate", candidates[0].(neotest.SingleSigner).Account().PublicKey().Bytes()) txes = append(txes, registerTx) - voteTx := neoValidatorsInvoker.WithSigners(voters[0]).PrepareInvoke(t, "vote", voters[0].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[0].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + voteTx := neoValidatorsInvoker.WithSigners(voters[0]).PrepareInvoke(t, "vote", voters[0].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[0].(neotest.SingleSigner).Account().PublicKey().Bytes()) txes = append(txes, voteTx) neoValidatorsInvoker.AddNewBlock(t, txes...) for _, tx := range txes { @@ -198,7 +198,7 @@ func TestNEO_Vote(t *testing.T) { require.NoError(t, err) sortedCandidates := make(keys.PublicKeys, validatorsCount) for i := range candidates[:validatorsCount] { - sortedCandidates[i] = candidates[i].(neotest.SingleSigner).Account().PrivateKey().PublicKey() + sortedCandidates[i] = candidates[i].(neotest.SingleSigner).Account().PublicKey() } sort.Sort(sortedCandidates) require.EqualValues(t, sortedCandidates, keys.PublicKeys(pubs)) @@ -259,8 +259,8 @@ func TestNEO_Vote(t *testing.T) { } }) - neoCommitteeInvoker.WithSigners(candidates[0]).Invoke(t, true, "unregisterCandidate", candidates[0].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) - neoCommitteeInvoker.WithSigners(voters[0]).Invoke(t, false, "vote", voters[0].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[0].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + neoCommitteeInvoker.WithSigners(candidates[0]).Invoke(t, true, "unregisterCandidate", candidates[0].(neotest.SingleSigner).Account().PublicKey().Bytes()) + neoCommitteeInvoker.WithSigners(voters[0]).Invoke(t, false, "vote", voters[0].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[0].(neotest.SingleSigner).Account().PublicKey().Bytes()) advanceChain(t) @@ -564,9 +564,9 @@ func TestNEO_GetCandidates(t *testing.T) { for i := 0; i < candidatesCount; i++ { transferTx := neoValidatorsInvoker.PrepareInvoke(t, "transfer", e.Validator.ScriptHash(), voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), int64(candidatesCount+1-i)*1000000, nil) txes = append(txes, transferTx) - registerTx := neoValidatorsInvoker.WithSigners(candidates[i]).PrepareInvoke(t, "registerCandidate", candidates[i].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + registerTx := neoValidatorsInvoker.WithSigners(candidates[i]).PrepareInvoke(t, "registerCandidate", candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes()) txes = append(txes, registerTx) - voteTx := neoValidatorsInvoker.WithSigners(voters[i]).PrepareInvoke(t, "vote", voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[i].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()) + voteTx := neoValidatorsInvoker.WithSigners(voters[i]).PrepareInvoke(t, "vote", voters[i].(neotest.SingleSigner).Account().PrivateKey().GetScriptHash(), candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes()) txes = append(txes, voteTx) } @@ -576,7 +576,7 @@ func TestNEO_GetCandidates(t *testing.T) { } expected := make([]stackitem.Item, candidatesCount) for i := range expected { - pub := candidates[i].(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes() + pub := candidates[i].(neotest.SingleSigner).Account().PublicKey().Bytes() v := stackitem.NewBigInteger(big.NewInt(int64(candidatesCount-i+1) * 1000000)) expected[i] = stackitem.NewStruct([]stackitem.Item{ stackitem.NewByteArray(pub), @@ -613,7 +613,7 @@ func TestNEO_GetCandidates(t *testing.T) { checkGetAllCandidates(t, expected) // Block candidate and check it won't be returned from getCandidates and getAllCandidates. - unlucky := candidates[len(candidates)-1].(neotest.SingleSigner).Account().PrivateKey().PublicKey() + unlucky := candidates[len(candidates)-1].(neotest.SingleSigner).Account().PublicKey() policyInvoker.Invoke(t, true, "blockAccount", unlucky.GetScriptHash()) for i := range expected { if bytes.Equal(expected[i].Value().([]stackitem.Item)[0].Value().([]byte), unlucky.Bytes()) { diff --git a/pkg/core/native/native_test/oracle_test.go b/pkg/core/native/native_test/oracle_test.go index dec7ab41d..f7ce224f3 100644 --- a/pkg/core/native/native_test/oracle_test.go +++ b/pkg/core/native/native_test/oracle_test.go @@ -71,8 +71,8 @@ func TestOracle_Request(t *testing.T) { // Designate single Oracle node. oracleNode := e.NewAccount(t) - designationCommitteeInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", int(noderoles.Oracle), []interface{}{oracleNode.(neotest.SingleSigner).Account().PrivateKey().PublicKey().Bytes()}) - err = oracleNode.(neotest.SingleSigner).Account().ConvertMultisig(1, []*keys.PublicKey{oracleNode.(neotest.SingleSigner).Account().PrivateKey().PublicKey()}) + designationCommitteeInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", int(noderoles.Oracle), []interface{}{oracleNode.(neotest.SingleSigner).Account().PublicKey().Bytes()}) + err = oracleNode.(neotest.SingleSigner).Account().ConvertMultisig(1, []*keys.PublicKey{oracleNode.(neotest.SingleSigner).Account().PublicKey()}) require.NoError(t, err) oracleNodeMulti := neotest.NewMultiSigner(oracleNode.(neotest.SingleSigner).Account()) gasCommitteeInvoker.Invoke(t, true, "transfer", gasCommitteeInvoker.CommitteeHash, oracleNodeMulti.ScriptHash(), 100_0000_0000, nil) diff --git a/pkg/neotest/chain/chain.go b/pkg/neotest/chain/chain.go index 1d675687d..4e6de45e0 100644 --- a/pkg/neotest/chain/chain.go +++ b/pkg/neotest/chain/chain.go @@ -58,7 +58,7 @@ var ( func init() { committeeAcc, _ = wallet.NewAccountFromWIF(singleValidatorWIF) - pubs := keys.PublicKeys{committeeAcc.PrivateKey().PublicKey()} + pubs := keys.PublicKeys{committeeAcc.PublicKey()} err := committeeAcc.ConvertMultisig(1, pubs) if err != nil { panic(err) @@ -70,7 +70,7 @@ func init() { pubs = make(keys.PublicKeys, len(accs)) for i := range committeeWIFs { accs[i], _ = wallet.NewAccountFromWIF(committeeWIFs[i]) - pubs[i] = accs[i].PrivateKey().PublicKey() + pubs[i] = accs[i].PublicKey() } // Config entry must contain validators first in a specific order. @@ -86,8 +86,8 @@ func init() { sort.Sort(pubs[:4]) sort.Slice(accs[:4], func(i, j int) bool { - p1 := accs[i].PrivateKey().PublicKey() - p2 := accs[j].PrivateKey().PublicKey() + p1 := accs[i].PublicKey() + p2 := accs[j].PublicKey() return p1.Cmp(p2) == -1 }) for i := range multiValidatorAcc { @@ -102,8 +102,8 @@ func init() { sort.Sort(pubs) sort.Slice(accs, func(i, j int) bool { - p1 := accs[i].PrivateKey().PublicKey() - p2 := accs[j].PrivateKey().PublicKey() + p1 := accs[i].PublicKey() + p2 := accs[j].PublicKey() return p1.Cmp(p2) == -1 }) for i := range multiCommitteeAcc { @@ -141,7 +141,7 @@ func NewSingleWithCustomConfigAndStore(t testing.TB, f func(cfg *config.Protocol Magic: netmode.UnitTestNet, MaxTraceableBlocks: MaxTraceableBlocks, SecondsPerBlock: SecondsPerBlock, - StandbyCommittee: []string{hex.EncodeToString(committeeAcc.PrivateKey().PublicKey().Bytes())}, + StandbyCommittee: []string{hex.EncodeToString(committeeAcc.PublicKey().Bytes())}, ValidatorsCount: 1, VerifyBlocks: true, VerifyTransactions: true, diff --git a/pkg/neotest/signer.go b/pkg/neotest/signer.go index 04020554b..5821e4803 100644 --- a/pkg/neotest/signer.go +++ b/pkg/neotest/signer.go @@ -103,8 +103,8 @@ func NewMultiSigner(accs ...*wallet.Account) MultiSigner { "but only %d accounts were provided", m, len(accs))) } sort.Slice(accs, func(i, j int) bool { - p1 := accs[i].PrivateKey().PublicKey() - p2 := accs[j].PrivateKey().PublicKey() + p1 := accs[i].PublicKey() + p2 := accs[j].PublicKey() return p1.Cmp(p2) == -1 }) for _, acc := range accs { diff --git a/pkg/neotest/signer_test.go b/pkg/neotest/signer_test.go index 3b4f27cc6..0f14a1512 100644 --- a/pkg/neotest/signer_test.go +++ b/pkg/neotest/signer_test.go @@ -28,7 +28,7 @@ func TestMultiSigner(t *testing.T) { require.NoError(t, err) accs[i] = a - pubs[i] = a.PrivateKey().PublicKey() + pubs[i] = a.PublicKey() } sort.Sort(pubs) @@ -40,8 +40,8 @@ func TestMultiSigner(t *testing.T) { s := NewMultiSigner(accs...) for i := range pubs { for j := range accs { - if pub := accs[j].PrivateKey().PublicKey(); pub.Equal(pubs[i]) { - require.Equal(t, pub, s.Single(i).Account().PrivateKey().PublicKey()) + if pub := accs[j].PublicKey(); pub.Equal(pubs[i]) { + require.Equal(t, pub, s.Single(i).Account().PublicKey()) } } } diff --git a/pkg/services/notary/core_test.go b/pkg/services/notary/core_test.go index 3aaeb7837..448f06b14 100644 --- a/pkg/services/notary/core_test.go +++ b/pkg/services/notary/core_test.go @@ -154,7 +154,7 @@ func TestNotary(t *testing.T) { mp1.StopSubscriptions() }) - notaryNodes := []interface{}{acc1.PrivateKey().PublicKey().Bytes(), acc2.PrivateKey().PublicKey().Bytes()} + notaryNodes := []interface{}{acc1.PublicKey().Bytes(), acc2.PrivateKey().PublicKey().Bytes()} designationSuperInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", int64(noderoles.P2PNotary), notaryNodes) @@ -175,7 +175,7 @@ func TestNotary(t *testing.T) { Scopes: transaction.None, }, { - Account: requester.PrivateKey().PublicKey().GetScriptHash(), + Account: requester.PublicKey().GetScriptHash(), Scopes: transaction.None, }, } @@ -222,12 +222,12 @@ func TestNotary(t *testing.T) { var script []byte switch requesters[i].typ { case notary.Signature: - script = requesters[i].accounts[0].PrivateKey().PublicKey().GetVerificationScript() + script = requesters[i].accounts[0].PublicKey().GetVerificationScript() nKeys++ case notary.MultiSignature: pubs := make(keys.PublicKeys, len(requesters[i].accounts)) for j, r := range requesters[i].accounts { - pubs[j] = r.PrivateKey().PublicKey() + pubs[j] = r.PublicKey() } script, err = smartcontract.CreateMultiSigRedeemScript(requesters[i].m, pubs) require.NoError(t, err) @@ -456,7 +456,7 @@ func TestNotary(t *testing.T) { r, _ := checkCompleteStandardRequest(t, 1, false) checkFallbackTxs(t, r, false) // set account back for the next tests - ntr1.UpdateNotaryNodes(keys.PublicKeys{acc1.PrivateKey().PublicKey()}) + ntr1.UpdateNotaryNodes(keys.PublicKeys{acc1.PublicKey()}) // OnNewRequest: signature request for _, i := range []int{1, 2, 3, 10} { @@ -496,7 +496,7 @@ func TestNotary(t *testing.T) { checkMainTx(t, requesters, r, 1, false) checkFallbackTxs(t, r, false) // set account back for the next tests - ntr1.UpdateNotaryNodes(keys.PublicKeys{acc1.PrivateKey().PublicKey()}) + ntr1.UpdateNotaryNodes(keys.PublicKeys{acc1.PublicKey()}) // PostPersist: complete main transaction, signature request setFinalizeWithError(true) @@ -634,7 +634,7 @@ func TestNotary(t *testing.T) { checkMainTx(t, requesters, requests, len(requests), false) checkFallbackTxs(t, requests, false) // set account back for the next tests - ntr1.UpdateNotaryNodes(keys.PublicKeys{acc1.PrivateKey().PublicKey()}) + ntr1.UpdateNotaryNodes(keys.PublicKeys{acc1.PublicKey()}) // OnRequestRemoval: signature request, remove one fallback // check OnNewRequest with finalization error @@ -721,9 +721,9 @@ func TestNotary(t *testing.T) { requester1, _ := wallet.NewAccount() requester2, _ := wallet.NewAccount() amount := int64(100_0000_0000) - gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester1.PrivateKey().PublicKey().GetScriptHash(), int64(bc.BlockHeight() + 50)}) + gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester1.PublicKey().GetScriptHash(), int64(bc.BlockHeight() + 50)}) e.CheckGASBalance(t, notaryHash, big.NewInt(amount)) - gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester2.PrivateKey().PublicKey().GetScriptHash(), int64(bc.BlockHeight() + 50)}) + gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester2.PublicKey().GetScriptHash(), int64(bc.BlockHeight() + 50)}) e.CheckGASBalance(t, notaryHash, big.NewInt(2*amount)) // create request for 2 standard signatures => main tx should be completed after the second request is added to the pool diff --git a/pkg/services/notary/node.go b/pkg/services/notary/node.go index ea179baaa..9a7486c56 100644 --- a/pkg/services/notary/node.go +++ b/pkg/services/notary/node.go @@ -15,7 +15,7 @@ func (n *Notary) UpdateNotaryNodes(notaryNodes keys.PublicKeys) { if n.currAccount != nil { for _, node := range notaryNodes { - if node.Equal(n.currAccount.PrivateKey().PublicKey()) { + if node.Equal(n.currAccount.PublicKey()) { return } } diff --git a/pkg/services/notary/node_test.go b/pkg/services/notary/node_test.go index 979c3e848..b1779cd6c 100644 --- a/pkg/services/notary/node_test.go +++ b/pkg/services/notary/node_test.go @@ -44,11 +44,11 @@ func TestUpdateNotaryNodes(t *testing.T) { // currAcc is nil before UpdateNotaryNodes call require.Nil(t, ntr.currAccount) // set account for the first time - ntr.UpdateNotaryNodes(keys.PublicKeys{acc.PrivateKey().PublicKey()}) + ntr.UpdateNotaryNodes(keys.PublicKeys{acc.PublicKey()}) require.Equal(t, acc, ntr.currAccount) t.Run("account is already set", func(t *testing.T) { - ntr.UpdateNotaryNodes(keys.PublicKeys{acc.PrivateKey().PublicKey(), randomKey.PublicKey()}) + ntr.UpdateNotaryNodes(keys.PublicKeys{acc.PublicKey(), randomKey.PublicKey()}) require.Equal(t, acc, ntr.currAccount) }) @@ -57,14 +57,14 @@ func TestUpdateNotaryNodes(t *testing.T) { w, err := wallet.NewWalletFromFile("./testdata/notary1.json") require.NoError(t, err) require.NoError(t, w.Accounts[1].Decrypt("one", w.Scrypt)) - ntr.UpdateNotaryNodes(keys.PublicKeys{w.Accounts[1].PrivateKey().PublicKey()}) + ntr.UpdateNotaryNodes(keys.PublicKeys{w.Accounts[1].PublicKey()}) require.Equal(t, w.Accounts[1], ntr.currAccount) }) t.Run("bad config password", func(t *testing.T) { w, err := wallet.NewWalletFromFile("./testdata/notary1.json") require.NoError(t, err) require.NoError(t, w.Accounts[2].Decrypt("four", w.Scrypt)) - ntr.UpdateNotaryNodes(keys.PublicKeys{w.Accounts[2].PrivateKey().PublicKey()}) + ntr.UpdateNotaryNodes(keys.PublicKeys{w.Accounts[2].PublicKey()}) require.Nil(t, ntr.currAccount) }) }) diff --git a/pkg/services/oracle/oracle_test.go b/pkg/services/oracle/oracle_test.go index f30aebbb1..9cc43bcee 100644 --- a/pkg/services/oracle/oracle_test.go +++ b/pkg/services/oracle/oracle_test.go @@ -122,7 +122,7 @@ func TestCreateResponseTx(t *testing.T) { } cInvoker.Invoke(t, stackitem.Null{}, "requestURL", req.URL, *req.Filter, req.CallbackMethod, req.UserData, int64(req.GasForResponse)) bc.SetOracle(orc) - orc.UpdateOracleNodes(keys.PublicKeys{acc.PrivateKey().PublicKey()}) + orc.UpdateOracleNodes(keys.PublicKeys{acc.PublicKey()}) tx, err = orc.CreateResponseTx(int64(req.GasForResponse), 1, resp) require.NoError(t, err) assert.Equal(t, 166, tx.Size()) @@ -150,7 +150,7 @@ func TestOracle(t *testing.T) { acc1, orc1, m1, ch1 := getTestOracle(t, bc, "./testdata/oracle1.json", "one") acc2, orc2, m2, ch2 := getTestOracle(t, bc, "./testdata/oracle2.json", "two") - oracleNodes := keys.PublicKeys{acc1.PrivateKey().PublicKey(), acc2.PrivateKey().PublicKey()} + oracleNodes := keys.PublicKeys{acc1.PublicKey(), acc2.PrivateKey().PublicKey()} // Must be set in native contract for tx verification. designationSuperInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", int64(roles.Oracle), []interface{}{oracleNodes[0].Bytes(), oracleNodes[1].Bytes()}) @@ -249,10 +249,10 @@ func TestOracle(t *testing.T) { require.Empty(t, ch2) t.Run("InvalidSignature", func(t *testing.T) { - orc1.AddResponse(acc2.PrivateKey().PublicKey(), m2[0].resp.ID, []byte{1, 2, 3}) + orc1.AddResponse(acc2.PublicKey(), m2[0].resp.ID, []byte{1, 2, 3}) require.Empty(t, ch1) }) - orc1.AddResponse(acc2.PrivateKey().PublicKey(), m2[0].resp.ID, m2[0].txSig) + orc1.AddResponse(acc2.PublicKey(), m2[0].resp.ID, m2[0].txSig) checkEmitTx(t, ch1) t.Run("FirstOtherThenMe", func(t *testing.T) { @@ -264,7 +264,7 @@ func TestOracle(t *testing.T) { Result: []byte{1, 2, 3, 4}, } req := checkResp(t, reqID, resp) - orc2.AddResponse(acc1.PrivateKey().PublicKey(), reqID, m1[reqID].txSig) + orc2.AddResponse(acc1.PublicKey(), reqID, m1[reqID].txSig) require.Empty(t, ch2) reqs := map[uint64]*state.OracleRequest{reqID: req} @@ -357,7 +357,7 @@ func TestOracleFull(t *testing.T) { }) designationSuperInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", - int64(roles.Oracle), []interface{}{acc.PrivateKey().PublicKey().Bytes()}) + int64(roles.Oracle), []interface{}{acc.PublicKey().Bytes()}) cs := contracts.GetOracleContractState(t, pathToInternalContracts, validator.ScriptHash(), 0) e.DeployContract(t, &neotest.Contract{ @@ -391,7 +391,7 @@ func TestNotYetRunningOracle(t *testing.T) { t.Cleanup(bc.Close) designationSuperInvoker.Invoke(t, stackitem.Null{}, "designateAsRole", - int64(roles.Oracle), []interface{}{acc.PrivateKey().PublicKey().Bytes()}) + int64(roles.Oracle), []interface{}{acc.PublicKey().Bytes()}) var req state.OracleRequest var reqs = make(map[uint64]*state.OracleRequest) diff --git a/pkg/services/stateroot/service_test.go b/pkg/services/stateroot/service_test.go index 047138c76..92df60ad6 100644 --- a/pkg/services/stateroot/service_test.go +++ b/pkg/services/stateroot/service_test.go @@ -65,13 +65,13 @@ func newMajorityMultisigWithGAS(t *testing.T, n int) (util.Uint160, keys.PublicK accs[i] = acc } sort.Slice(accs, func(i, j int) bool { - pi := accs[i].PrivateKey().PublicKey() - pj := accs[j].PrivateKey().PublicKey() + pi := accs[i].PublicKey() + pj := accs[j].PublicKey() return pi.Cmp(pj) == -1 }) pubs := make(keys.PublicKeys, n) for i := range pubs { - pubs[i] = accs[i].PrivateKey().PublicKey() + pubs[i] = accs[i].PublicKey() } script, err := smartcontract.CreateMajorityMultiSigRedeemScript(pubs) require.NoError(t, err) @@ -126,7 +126,7 @@ func TestStateRoot(t *testing.T) { t.Run("invalid signer", func(t *testing.T) { accInv, err := wallet.NewAccount() require.NoError(t, err) - pubs := keys.PublicKeys{accInv.PrivateKey().PublicKey()} + pubs := keys.PublicKeys{accInv.PublicKey()} require.NoError(t, accInv.ConvertMultisig(1, pubs)) gasValidatorInvoker.Invoke(t, true, "transfer", validator.ScriptHash(), accInv.Contract.ScriptHash(), 1_0000_0000, nil) r, err := bc.GetStateModule().GetStateRoot(1) diff --git a/pkg/services/stateroot/validators.go b/pkg/services/stateroot/validators.go index 5a3ce73a4..c0742d4f6 100644 --- a/pkg/services/stateroot/validators.go +++ b/pkg/services/stateroot/validators.go @@ -91,7 +91,7 @@ func (s *service) signAndSend(r *state.MPTRoot) error { incRoot.Lock() defer incRoot.Unlock() incRoot.root = r - incRoot.addSignature(acc.PrivateKey().PublicKey(), sig) + incRoot.addSignature(acc.PublicKey(), sig) incRoot.reverify(s.Network) s.trySendRoot(incRoot, acc) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index c0bc5910a..15df77eac 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -140,7 +140,7 @@ func (a *Account) GetVerificationScript() []byte { if a.Contract != nil { return a.Contract.Script } - return a.PrivateKey().PublicKey().GetVerificationScript() + return a.privateKey.PublicKey().GetVerificationScript() } // Decrypt decrypts the EncryptedWIF with the given passphrase returning error @@ -175,6 +175,15 @@ func (a *Account) PrivateKey() *keys.PrivateKey { return a.privateKey } +// PublicKey returns the public key associated with the private key corresponding to +// the account. It can return nil if account is locked (use CanSign to check). +func (a *Account) PublicKey() *keys.PublicKey { + if !a.CanSign() { + return nil + } + return a.privateKey.PublicKey() +} + // Close cleans up the private key used by Account and disassociates it from // Account. The Account can no longer sign anything after this call, but Decrypt // can make it usable again. diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 238430217..88118176e 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -105,7 +105,7 @@ func TestContractSignTx(t *testing.T) { require.Error(t, acc2.SignTx(0, tx)) - pubs := keys.PublicKeys{acc.privateKey.PublicKey(), acc2.privateKey.PublicKey()} + pubs := keys.PublicKeys{acc.PublicKey(), acc2.PublicKey()} multiS, err := smartcontract.CreateDefaultMultiSigRedeemScript(pubs) require.NoError(t, err) multiAcc := NewAccountFromPrivateKey(acc.privateKey) @@ -139,6 +139,7 @@ func TestContractSignTx(t *testing.T) { acc2.Locked = true require.False(t, acc2.CanSign()) require.Error(t, acc2.SignTx(0, tx)) // Locked account. + require.Nil(t, acc2.PublicKey()) // Locked account. acc2.Locked = false acc2.Close() @@ -232,7 +233,7 @@ func compareFields(t *testing.T, tk keytestcases.Ktype, acc *Account) { require.Equalf(t, want, have, "expected address %s got %s", want, have) want, have = tk.Wif, acc.privateKey.WIF() require.Equalf(t, want, have, "expected wif %s got %s", want, have) - want, have = tk.PublicKey, hex.EncodeToString(acc.privateKey.PublicKey().Bytes()) + want, have = tk.PublicKey, hex.EncodeToString(acc.PublicKey().Bytes()) require.Equalf(t, want, have, "expected pub key %s got %s", want, have) want, have = tk.PrivateKey, acc.privateKey.String() require.Equalf(t, want, have, "expected priv key %s got %s", want, have) From fd8da6fdb9ef3a6ab09af95a669a65565748c1f4 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 17:54:00 +0300 Subject: [PATCH 06/15] *: do not get private key from Account to check if it CanSign() We have this API now to performs checks. --- cli/smartcontract/smart_contract.go | 2 +- pkg/consensus/consensus.go | 2 +- pkg/services/notary/node.go | 2 +- pkg/services/oracle/nodes.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index f96935d10..fd39fba8f 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -892,7 +892,7 @@ func getUnlockedAccount(wall *wallet.Wallet, addr util.Uint160, pass *string) (* return nil, fmt.Errorf("wallet contains no account for '%s'", address.Uint160ToString(addr)) } - if acc.PrivateKey() != nil { + if acc.CanSign() { return acc, nil } diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index 38326225a..7e8523ebd 100644 --- a/pkg/consensus/consensus.go +++ b/pkg/consensus/consensus.go @@ -377,7 +377,7 @@ func (s *service) getKeyPair(pubs []crypto.PublicKey) (int, crypto.PrivateKey, c continue } - if acc.PrivateKey() == nil { + if !acc.CanSign() { err := acc.Decrypt(s.Config.Wallet.Password, s.wallet.Scrypt) if err != nil { s.log.Fatal("can't unlock account", zap.String("address", address.Uint160ToString(sh))) diff --git a/pkg/services/notary/node.go b/pkg/services/notary/node.go index 9a7486c56..341131006 100644 --- a/pkg/services/notary/node.go +++ b/pkg/services/notary/node.go @@ -25,7 +25,7 @@ func (n *Notary) UpdateNotaryNodes(notaryNodes keys.PublicKeys) { for _, node := range notaryNodes { acc = n.wallet.GetAccount(node.GetScriptHash()) if acc != nil { - if acc.PrivateKey() != nil { + if acc.CanSign() { break } err := acc.Decrypt(n.Config.MainCfg.UnlockWallet.Password, n.wallet.Scrypt) diff --git a/pkg/services/oracle/nodes.go b/pkg/services/oracle/nodes.go index b20b806c0..0331de62d 100644 --- a/pkg/services/oracle/nodes.go +++ b/pkg/services/oracle/nodes.go @@ -30,7 +30,7 @@ func (o *Oracle) UpdateOracleNodes(oracleNodes keys.PublicKeys) { for i := range oracleNodes { acc = o.wallet.GetAccount(oracleNodes[i].GetScriptHash()) if acc != nil { - if acc.PrivateKey() != nil { + if acc.CanSign() { break } err := acc.Decrypt(o.MainCfg.UnlockWallet.Password, o.wallet.Scrypt) From e569edc84149dc732386033614dc333f893e433e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 19:04:47 +0300 Subject: [PATCH 07/15] wallet: add ScriptHash() to Account It allows to simplify a lot of code and avoid getting a PrivateKey in some cases. --- cli/cmdargs/parser.go | 5 +---- cli/nep17_test.go | 7 +------ cli/paramcontext/context.go | 7 +------ cli/wallet/nep17.go | 7 +------ pkg/core/native/native_test/neo_test.go | 2 +- pkg/rpcclient/actor/actor_test.go | 6 +----- pkg/rpcclient/nep11.go | 13 ++---------- pkg/rpcclient/nep17.go | 6 +----- pkg/rpcclient/rpc.go | 10 ++------- pkg/services/notary/core_test.go | 6 +++--- pkg/services/rpcsrv/client_test.go | 5 ----- pkg/services/stateroot/network.go | 5 ++--- pkg/services/stateroot/validators.go | 2 +- pkg/wallet/account.go | 28 ++++++++++++++++--------- pkg/wallet/account_test.go | 2 ++ pkg/wallet/wallet_test.go | 1 + 16 files changed, 38 insertions(+), 74 deletions(-) diff --git a/cli/cmdargs/parser.go b/cli/cmdargs/parser.go index 2661d5831..ab8f8a34b 100644 --- a/cli/cmdargs/parser.go +++ b/cli/cmdargs/parser.go @@ -199,10 +199,7 @@ func ParseParams(args []string, calledFromMain bool) (int, []smartcontract.Param // accounts from the provided wallet. func GetSignersAccounts(senderAcc *wallet.Account, wall *wallet.Wallet, signers []transaction.Signer, accScope transaction.WitnessScope) ([]actor.SignerAccount, error) { signersAccounts := make([]actor.SignerAccount, 0, len(signers)+1) - sender, err := address.StringToUint160(senderAcc.Address) - if err != nil { - return nil, err - } + sender := senderAcc.ScriptHash() signersAccounts = append(signersAccounts, actor.SignerAccount{ Signer: transaction.Signer{ Account: sender, diff --git a/cli/nep17_test.go b/cli/nep17_test.go index b318616d1..6a43f96e6 100644 --- a/cli/nep17_test.go +++ b/cli/nep17_test.go @@ -162,8 +162,7 @@ func TestNEP17Transfer(t *testing.T) { e.checkNextLine(t, `^Total fee:\s*(\d|\.)+`) e.checkTxPersisted(t) - sh, err := address.StringToUint160(w.Accounts[0].Address) - require.NoError(t, err) + sh := w.Accounts[0].ScriptHash() b, _ := e.Chain.GetGoverningTokenBalance(sh) require.Equal(t, big.NewInt(1), b) @@ -172,8 +171,6 @@ func TestNEP17Transfer(t *testing.T) { e.Run(t, append(args, "--force")...) e.checkTxPersisted(t) - sh, err := address.StringToUint160(w.Accounts[0].Address) - require.NoError(t, err) b, _ := e.Chain.GetGoverningTokenBalance(sh) require.Equal(t, big.NewInt(2), b) }) @@ -198,8 +195,6 @@ func TestNEP17Transfer(t *testing.T) { e.Run(t, args...) e.checkTxPersisted(t) - sh, err := address.StringToUint160(w.Accounts[0].Address) - require.NoError(t, err) b, _ := e.Chain.GetGoverningTokenBalance(sh) require.Equal(t, big.NewInt(3), b) diff --git a/cli/paramcontext/context.go b/cli/paramcontext/context.go index 85175d4d2..bd1b8d5d9 100644 --- a/cli/paramcontext/context.go +++ b/cli/paramcontext/context.go @@ -7,7 +7,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/transaction" - "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/smartcontract/context" "github.com/nspcc-dev/neo-go/pkg/wallet" ) @@ -21,11 +20,7 @@ func InitAndSave(net netmode.Magic, tx *transaction.Transaction, acc *wallet.Acc priv := acc.PrivateKey() pub := priv.PublicKey() sign := priv.SignHashable(uint32(net), tx) - h, err := address.StringToUint160(acc.Address) - if err != nil { - return fmt.Errorf("invalid address: %s", acc.Address) - } - if err := scCtx.AddSignature(h, acc.Contract, pub, sign); err != nil { + if err := scCtx.AddSignature(acc.ScriptHash(), acc.Contract, pub, sign); err != nil { return fmt.Errorf("can't add signature: %w", err) } } diff --git a/cli/wallet/nep17.go b/cli/wallet/nep17.go index 80623528b..94077b9ab 100644 --- a/cli/wallet/nep17.go +++ b/cli/wallet/nep17.go @@ -286,17 +286,12 @@ func getNEPBalance(ctx *cli.Context, standard string, accHandler func(*cli.Conte } } for k, acc := range accounts { - addrHash, err := address.StringToUint160(acc.Address) - if err != nil { - return cli.NewExitError(fmt.Errorf("invalid account address: %w", err), 1) - } - if k != 0 { fmt.Fprintln(ctx.App.Writer) } fmt.Fprintf(ctx.App.Writer, "Account %s\n", acc.Address) - err = accHandler(ctx, c, addrHash, name, token, tokenID) + err = accHandler(ctx, c, acc.ScriptHash(), name, token, tokenID) if err != nil { return cli.NewExitError(err, 1) } diff --git a/pkg/core/native/native_test/neo_test.go b/pkg/core/native/native_test/neo_test.go index ef9e62433..011636ca9 100644 --- a/pkg/core/native/native_test/neo_test.go +++ b/pkg/core/native/native_test/neo_test.go @@ -166,7 +166,7 @@ func TestNEO_Vote(t *testing.T) { txes = append(txes, voteTx) } } - txes = append(txes, policyInvoker.PrepareInvoke(t, "blockAccount", candidates[len(candidates)-1].(neotest.SingleSigner).Account().PublicKey().GetScriptHash())) + txes = append(txes, policyInvoker.PrepareInvoke(t, "blockAccount", candidates[len(candidates)-1].(neotest.SingleSigner).Account().ScriptHash())) neoValidatorsInvoker.AddNewBlock(t, txes...) for _, tx := range txes { e.CheckHalt(t, tx.Hash(), stackitem.Make(true)) // luckily, both `transfer`, `registerCandidate` and `vote` return boolean values diff --git a/pkg/rpcclient/actor/actor_test.go b/pkg/rpcclient/actor/actor_test.go index 5ec1b4c4f..d621d2770 100644 --- a/pkg/rpcclient/actor/actor_test.go +++ b/pkg/rpcclient/actor/actor_test.go @@ -7,7 +7,6 @@ import ( "github.com/google/uuid" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/transaction" - "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/neorpc/result" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" @@ -276,8 +275,5 @@ func TestSender(t *testing.T) { client, acc := testRPCAndAccount(t) a, err := NewSimple(client, acc) require.NoError(t, err) - - addr, err := address.StringToUint160(acc.Address) - require.NoError(t, err) - require.Equal(t, addr, a.Sender()) + require.Equal(t, acc.ScriptHash(), a.Sender()) } diff --git a/pkg/rpcclient/nep11.go b/pkg/rpcclient/nep11.go index 1c16990e9..723922759 100644 --- a/pkg/rpcclient/nep11.go +++ b/pkg/rpcclient/nep11.go @@ -6,7 +6,6 @@ import ( "github.com/google/uuid" "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/core/transaction" - "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/neorpc/result" "github.com/nspcc-dev/neo-go/pkg/rpcclient/unwrap" "github.com/nspcc-dev/neo-go/pkg/smartcontract" @@ -88,13 +87,9 @@ func (c *Client) CreateNEP11TransferTx(acc *wallet.Account, tokenHash util.Uint1 if err != nil { return nil, fmt.Errorf("failed to create NEP-11 transfer script: %w", err) } - from, err := address.StringToUint160(acc.Address) - if err != nil { - return nil, fmt.Errorf("bad account address: %w", err) - } return c.CreateTxFromScript(script, acc, -1, gas, append([]SignerAccount{{ Signer: transaction.Signer{ - Account: from, + Account: acc.ScriptHash(), Scopes: transaction.CalledByEntry, }, Account: acc, @@ -148,11 +143,7 @@ func (c *Client) NEP11NDOwnerOf(tokenHash util.Uint160, tokenID []byte) (util.Ui // versions. func (c *Client) TransferNEP11D(acc *wallet.Account, to util.Uint160, tokenHash util.Uint160, amount int64, tokenID []byte, data interface{}, gas int64, cosigners []SignerAccount) (util.Uint256, error) { - from, err := address.StringToUint160(acc.Address) - if err != nil { - return util.Uint256{}, fmt.Errorf("bad account address: %w", err) - } - tx, err := c.CreateNEP11TransferTx(acc, tokenHash, gas, cosigners, from, to, amount, tokenID, data) + tx, err := c.CreateNEP11TransferTx(acc, tokenHash, gas, cosigners, acc.ScriptHash(), to, amount, tokenID, data) if err != nil { return util.Uint256{}, err } diff --git a/pkg/rpcclient/nep17.go b/pkg/rpcclient/nep17.go index e1b988976..aa73a8c28 100644 --- a/pkg/rpcclient/nep17.go +++ b/pkg/rpcclient/nep17.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/nspcc-dev/neo-go/pkg/core/transaction" - "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/util" @@ -96,10 +95,7 @@ func (c *Client) CreateNEP17TransferTx(acc *wallet.Account, to util.Uint160, // be removed in future versions. func (c *Client) CreateNEP17MultiTransferTx(acc *wallet.Account, gas int64, recipients []TransferTarget, cosigners []SignerAccount) (*transaction.Transaction, error) { - from, err := address.StringToUint160(acc.Address) - if err != nil { - return nil, fmt.Errorf("bad account address: %w", err) - } + from := acc.ScriptHash() b := smartcontract.NewBuilder() for i := range recipients { b.InvokeWithAssert(recipients[i].Token, "transfer", diff --git a/pkg/rpcclient/rpc.go b/pkg/rpcclient/rpc.go index 4de2a57c0..cacbc4ea9 100644 --- a/pkg/rpcclient/rpc.go +++ b/pkg/rpcclient/rpc.go @@ -827,10 +827,7 @@ func getSigners(sender *wallet.Account, cosigners []SignerAccount) ([]transactio signers []transaction.Signer accounts []*wallet.Account ) - from, err := address.StringToUint160(sender.Address) - if err != nil { - return nil, nil, fmt.Errorf("bad sender account address: %v", err) - } + from := sender.ScriptHash() s := transaction.Signer{ Account: from, Scopes: transaction.None, @@ -875,10 +872,7 @@ func (c *Client) SignAndPushP2PNotaryRequest(mainTx *transaction.Transaction, fa if err != nil { return nil, fmt.Errorf("failed to get native Notary hash: %w", err) } - from, err := address.StringToUint160(acc.Address) - if err != nil { - return nil, fmt.Errorf("bad account address: %v", err) - } + from := acc.ScriptHash() signers := []transaction.Signer{{Account: notaryHash}, {Account: from}} if fallbackSysFee < 0 { result, err := c.InvokeScript(fallbackScript, signers) diff --git a/pkg/services/notary/core_test.go b/pkg/services/notary/core_test.go index 448f06b14..844aeab92 100644 --- a/pkg/services/notary/core_test.go +++ b/pkg/services/notary/core_test.go @@ -175,7 +175,7 @@ func TestNotary(t *testing.T) { Scopes: transaction.None, }, { - Account: requester.PublicKey().GetScriptHash(), + Account: requester.ScriptHash(), Scopes: transaction.None, }, } @@ -721,9 +721,9 @@ func TestNotary(t *testing.T) { requester1, _ := wallet.NewAccount() requester2, _ := wallet.NewAccount() amount := int64(100_0000_0000) - gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester1.PublicKey().GetScriptHash(), int64(bc.BlockHeight() + 50)}) + gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester1.ScriptHash(), int64(bc.BlockHeight() + 50)}) e.CheckGASBalance(t, notaryHash, big.NewInt(amount)) - gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester2.PublicKey().GetScriptHash(), int64(bc.BlockHeight() + 50)}) + gasValidatorInvoker.Invoke(t, true, "transfer", e.Validator.ScriptHash(), bc.GetNotaryContractScriptHash(), amount, []interface{}{requester2.ScriptHash(), int64(bc.BlockHeight() + 50)}) e.CheckGASBalance(t, notaryHash, big.NewInt(2*amount)) // create request for 2 standard signatures => main tx should be completed after the second request is added to the pool diff --git a/pkg/services/rpcsrv/client_test.go b/pkg/services/rpcsrv/client_test.go index dc7fb1119..b07b09455 100644 --- a/pkg/services/rpcsrv/client_test.go +++ b/pkg/services/rpcsrv/client_test.go @@ -1033,11 +1033,6 @@ func TestSignAndPushP2PNotaryRequest(t *testing.T) { }) require.NoError(t, c.Init()) - t.Run("bad account address", func(t *testing.T) { - _, err := c.SignAndPushP2PNotaryRequest(nil, nil, 0, 0, 0, &wallet.Account{Address: "not-an-addr"}) //nolint:staticcheck // SA1019: c.SignAndPushP2PNotaryRequest is deprecated - require.NotNil(t, err) - }) - t.Run("bad fallback script", func(t *testing.T) { _, err := c.SignAndPushP2PNotaryRequest(nil, []byte{byte(opcode.ASSERT)}, -1, 0, 0, acc) //nolint:staticcheck // SA1019: c.SignAndPushP2PNotaryRequest is deprecated require.NotNil(t, err) diff --git a/pkg/services/stateroot/network.go b/pkg/services/stateroot/network.go index e2eedcd8f..e91fb5100 100644 --- a/pkg/services/stateroot/network.go +++ b/pkg/services/stateroot/network.go @@ -90,7 +90,6 @@ func (s *service) trySendRoot(ir *incompleteRoot, acc *wallet.Account) { } func (s *service) sendValidatedRoot(r *state.MPTRoot, acc *wallet.Account) { - priv := acc.PrivateKey() w := io.NewBufBinWriter() m := NewMessage(RootT, r) m.EncodeBinary(w.BinWriter) @@ -98,13 +97,13 @@ func (s *service) sendValidatedRoot(r *state.MPTRoot, acc *wallet.Account) { Category: Category, ValidBlockStart: r.Index, ValidBlockEnd: r.Index + rootValidEndInc, - Sender: priv.GetScriptHash(), + Sender: acc.ScriptHash(), Data: w.Bytes(), Witness: transaction.Witness{ VerificationScript: acc.GetVerificationScript(), }, } - sig := priv.SignHashable(uint32(s.Network), ep) + sig := acc.PrivateKey().SignHashable(uint32(s.Network), ep) buf := io.NewBufBinWriter() emit.Bytes(buf.BinWriter, sig) ep.Witness.InvocationScript = buf.Bytes() diff --git a/pkg/services/stateroot/validators.go b/pkg/services/stateroot/validators.go index c0742d4f6..9bfe6e741 100644 --- a/pkg/services/stateroot/validators.go +++ b/pkg/services/stateroot/validators.go @@ -110,7 +110,7 @@ func (s *service) signAndSend(r *state.MPTRoot) error { Category: Category, ValidBlockStart: r.Index, ValidBlockEnd: r.Index + voteValidEndInc, - Sender: acc.PrivateKey().GetScriptHash(), + Sender: acc.ScriptHash(), Data: w.Bytes(), Witness: transaction.Witness{ VerificationScript: acc.GetVerificationScript(), diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 15df77eac..bd436d6ab 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -20,6 +20,9 @@ type Account struct { // NEO private key. privateKey *keys.PrivateKey + // Script hash corresponding to the Address. + scriptHash util.Uint160 + // NEO public address. Address string `json:"address"` @@ -80,8 +83,6 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { var ( haveAcc bool pos int - accHash util.Uint160 - err error ) if a.Locked { return errors.New("account is locked") @@ -89,12 +90,8 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { if a.Contract == nil { return errors.New("account has no contract") } - accHash, err = address.StringToUint160(a.Address) - if err != nil { - return err - } for i := range t.Signers { - if t.Signers[i].Account.Equals(accHash) { + if t.Signers[i].Account.Equals(a.ScriptHash()) { haveAcc = true pos = i break @@ -184,6 +181,16 @@ func (a *Account) PublicKey() *keys.PublicKey { return a.privateKey.PublicKey() } +// ScriptHash returns the script hash (account) that the Account.Address is +// derived from. It never returns an error, so if this Account has an invalid +// Address you'll just get a zero script hash. +func (a *Account) ScriptHash() util.Uint160 { + if a.scriptHash.Equals(util.Uint160{}) { + a.scriptHash, _ = address.StringToUint160(a.Address) + } + return a.scriptHash +} + // Close cleans up the private key used by Account and disassociates it from // Account. The Account can no longer sign anything after this call, but Decrypt // can make it usable again. @@ -243,7 +250,8 @@ func (a *Account) ConvertMultisig(m int, pubs []*keys.PublicKey) error { return err } - a.Address = address.Uint160ToString(hash.Hash160(script)) + a.scriptHash = hash.Hash160(script) + a.Address = address.Uint160ToString(a.scriptHash) a.Contract = &Contract{ Script: script, Parameters: getContractParams(m), @@ -255,11 +263,11 @@ func (a *Account) ConvertMultisig(m int, pubs []*keys.PublicKey) error { // NewAccountFromPrivateKey creates a wallet from the given PrivateKey. func NewAccountFromPrivateKey(p *keys.PrivateKey) *Account { pubKey := p.PublicKey() - pubAddr := p.Address() a := &Account{ privateKey: p, - Address: pubAddr, + scriptHash: p.GetScriptHash(), + Address: p.Address(), Contract: &Contract{ Script: pubKey.GetVerificationScript(), Parameters: getContractParams(1), diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 88118176e..714aa227e 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -9,6 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" + "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,6 +19,7 @@ func TestNewAccount(t *testing.T) { acc, err := NewAccount() require.NoError(t, err) require.NotNil(t, acc) + require.Equal(t, acc.Address, address.Uint160ToString(acc.ScriptHash())) } func TestDecryptAccount(t *testing.T) { diff --git a/pkg/wallet/wallet_test.go b/pkg/wallet/wallet_test.go index 7d155e9c4..9de064319 100644 --- a/pkg/wallet/wallet_test.go +++ b/pkg/wallet/wallet_test.go @@ -102,6 +102,7 @@ func TestSave(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, len(w2.Accounts)) require.NoError(t, w2.Accounts[1].Decrypt("pass", w2.Scrypt)) + _ = w2.Accounts[1].ScriptHash() // openedWallet has it for acc 1. require.Equal(t, openedWallet.Accounts, w2.Accounts) }) } From e164625a7ff43a5c322333f34ea7856719de070c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 20:42:42 +0300 Subject: [PATCH 08/15] wallet: provide (*Account).SignHashable API Make PrivateKey() less used and less useful. --- cli/paramcontext/context.go | 6 ++---- cli/wallet/multisig.go | 3 +-- pkg/neotest/signer.go | 4 ++-- pkg/rpcclient/notary/actor.go | 2 +- pkg/rpcclient/rpc.go | 2 +- pkg/services/notary/notary.go | 2 +- pkg/services/stateroot/network.go | 2 +- pkg/services/stateroot/validators.go | 4 ++-- pkg/wallet/account.go | 9 +++++++++ pkg/wallet/account_test.go | 6 ++++-- 10 files changed, 24 insertions(+), 16 deletions(-) diff --git a/cli/paramcontext/context.go b/cli/paramcontext/context.go index bd1b8d5d9..d8fe06999 100644 --- a/cli/paramcontext/context.go +++ b/cli/paramcontext/context.go @@ -17,10 +17,8 @@ import ( func InitAndSave(net netmode.Magic, tx *transaction.Transaction, acc *wallet.Account, filename string) error { scCtx := context.NewParameterContext("Neo.Network.P2P.Payloads.Transaction", net, tx) if acc != nil && acc.CanSign() { - priv := acc.PrivateKey() - pub := priv.PublicKey() - sign := priv.SignHashable(uint32(net), tx) - if err := scCtx.AddSignature(acc.ScriptHash(), acc.Contract, pub, sign); err != nil { + sign := acc.SignHashable(net, tx) + if err := scCtx.AddSignature(acc.ScriptHash(), acc.Contract, acc.PublicKey(), sign); err != nil { return fmt.Errorf("can't add signature: %w", err) } } diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index e489a42ef..8b691fd43 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -58,8 +58,7 @@ func signStoredTransaction(ctx *cli.Context) error { } if acc.CanSign() { - priv := acc.PrivateKey() - sign := priv.SignHashable(uint32(pc.Network), pc.Verifiable) + sign := acc.SignHashable(pc.Network, pc.Verifiable) if err := pc.AddSignature(ch, acc.Contract, acc.PublicKey(), sign); err != nil { return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) } diff --git a/pkg/neotest/signer.go b/pkg/neotest/signer.go index 5821e4803..df529e607 100644 --- a/pkg/neotest/signer.go +++ b/pkg/neotest/signer.go @@ -74,7 +74,7 @@ func (s *signer) ScriptHash() util.Uint160 { // SignHashable implements Signer interface. func (s *signer) SignHashable(magic uint32, item hash.Hashable) []byte { return append([]byte{byte(opcode.PUSHDATA1), 64}, - (*wallet.Account)(s).PrivateKey().SignHashable(magic, item)...) + (*wallet.Account)(s).SignHashable(netmode.Magic(magic), item)...) } // SignTx implements Signer interface. @@ -130,7 +130,7 @@ func (m multiSigner) Script() []byte { func (m multiSigner) SignHashable(magic uint32, item hash.Hashable) []byte { var script []byte for i := 0; i < m.m; i++ { - sign := m.accounts[i].PrivateKey().SignHashable(magic, item) + sign := m.accounts[i].SignHashable(netmode.Magic(magic), item) script = append(script, byte(opcode.PUSHDATA1), 64) script = append(script, sign...) } diff --git a/pkg/rpcclient/notary/actor.go b/pkg/rpcclient/notary/actor.go index b115c9b74..06c9ef958 100644 --- a/pkg/rpcclient/notary/actor.go +++ b/pkg/rpcclient/notary/actor.go @@ -300,7 +300,7 @@ func (a *Actor) SendRequestExactly(mainTx *transaction.Transaction, fbTx *transa FallbackTransaction: fbTx, } req.Witness = transaction.Witness{ - InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, a.sender.PrivateKey().SignHashable(uint32(a.GetNetwork()), req)...), + InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, a.sender.SignHashable(a.GetNetwork(), req)...), VerificationScript: a.sender.GetVerificationScript(), } actualHash, err := a.rpc.SubmitP2PNotaryRequest(req) diff --git a/pkg/rpcclient/rpc.go b/pkg/rpcclient/rpc.go index cacbc4ea9..e1cd3b52c 100644 --- a/pkg/rpcclient/rpc.go +++ b/pkg/rpcclient/rpc.go @@ -938,7 +938,7 @@ func (c *Client) SignAndPushP2PNotaryRequest(mainTx *transaction.Transaction, fa FallbackTransaction: fallbackTx, } req.Witness = transaction.Witness{ - InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, acc.PrivateKey().SignHashable(uint32(m), req)...), + InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, acc.SignHashable(m, req)...), VerificationScript: acc.GetVerificationScript(), } actualHash, err := c.SubmitP2PNotaryRequest(req) diff --git a/pkg/services/notary/notary.go b/pkg/services/notary/notary.go index fa3f3dd3f..f496902e9 100644 --- a/pkg/services/notary/notary.go +++ b/pkg/services/notary/notary.go @@ -391,7 +391,7 @@ func (n *Notary) PostPersist() { // finalize adds missing Notary witnesses to the transaction (main or fallback) and pushes it to the network. func (n *Notary) finalize(acc *wallet.Account, tx *transaction.Transaction, h util.Uint256) error { notaryWitness := transaction.Witness{ - InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, acc.PrivateKey().SignHashable(uint32(n.Network), tx)...), + InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, acc.SignHashable(n.Network, tx)...), VerificationScript: []byte{}, } for i, signer := range tx.Signers { diff --git a/pkg/services/stateroot/network.go b/pkg/services/stateroot/network.go index e91fb5100..cb586e2c0 100644 --- a/pkg/services/stateroot/network.go +++ b/pkg/services/stateroot/network.go @@ -103,7 +103,7 @@ func (s *service) sendValidatedRoot(r *state.MPTRoot, acc *wallet.Account) { VerificationScript: acc.GetVerificationScript(), }, } - sig := acc.PrivateKey().SignHashable(uint32(s.Network), ep) + sig := acc.SignHashable(s.Network, ep) buf := io.NewBufBinWriter() emit.Bytes(buf.BinWriter, sig) ep.Witness.InvocationScript = buf.Bytes() diff --git a/pkg/services/stateroot/validators.go b/pkg/services/stateroot/validators.go index 9bfe6e741..fc1bd1db1 100644 --- a/pkg/services/stateroot/validators.go +++ b/pkg/services/stateroot/validators.go @@ -86,7 +86,7 @@ func (s *service) signAndSend(r *state.MPTRoot) error { return nil } - sig := acc.PrivateKey().SignHashable(uint32(s.Network), r) + sig := acc.SignHashable(s.Network, r) incRoot := s.getIncompleteRoot(r.Index, myIndex) incRoot.Lock() defer incRoot.Unlock() @@ -116,7 +116,7 @@ func (s *service) signAndSend(r *state.MPTRoot) error { VerificationScript: acc.GetVerificationScript(), }, } - sig = acc.PrivateKey().SignHashable(uint32(s.Network), e) + sig = acc.SignHashable(s.Network, e) buf := io.NewBufBinWriter() emit.Bytes(buf.BinWriter, sig) e.Witness.InvocationScript = buf.Bytes() diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index bd436d6ab..2f5cb59f6 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -126,6 +126,15 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { return nil } +// SignHashable signs the given Hashable item and returns the signature. If this +// account can't sign (CanSign() returns false) nil is returned. +func (a *Account) SignHashable(net netmode.Magic, item hash.Hashable) []byte { + if !a.CanSign() { + return nil + } + return a.privateKey.SignHashable(uint32(net), item) +} + // CanSign returns true when account is not locked and has a decrypted private // key inside, so it's ready to create real signatures. func (a *Account) CanSign() bool { diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 714aa227e..a96340967 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -140,8 +140,9 @@ func TestContractSignTx(t *testing.T) { acc2.Locked = true require.False(t, acc2.CanSign()) - require.Error(t, acc2.SignTx(0, tx)) // Locked account. - require.Nil(t, acc2.PublicKey()) // Locked account. + require.Error(t, acc2.SignTx(0, tx)) // Locked account. + require.Nil(t, acc2.PublicKey()) // Locked account. + require.Nil(t, acc2.SignHashable(0, tx)) // Locked account. acc2.Locked = false acc2.Close() @@ -155,6 +156,7 @@ func TestContractSignTx(t *testing.T) { }) require.NoError(t, acc.SignTx(0, tx)) // Add invocation script for existing witness. require.Equal(t, 66, len(tx.Scripts[1].InvocationScript)) + require.NotNil(t, acc.SignHashable(0, tx)) // Works via Hashable too. require.NoError(t, multiAcc.SignTx(0, tx)) require.Equal(t, 3, len(tx.Scripts)) From cad0d7f00dce4f5fff4baae03b9328259176c284 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 21:03:37 +0300 Subject: [PATCH 09/15] wallet: add some warnings to Decrypt and PrivateKey docs --- pkg/wallet/account.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 2f5cb59f6..e1a7c30e3 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -150,7 +150,9 @@ func (a *Account) GetVerificationScript() []byte { } // Decrypt decrypts the EncryptedWIF with the given passphrase returning error -// if anything goes wrong. +// if anything goes wrong. After the decryption Account can be used to sign +// things unless it's locked. Don't decrypt the key unless you want to sign +// something and don't forget to call Close after use for maximum safety. func (a *Account) Decrypt(passphrase string, scrypt keys.ScryptParams) error { var err error @@ -176,7 +178,11 @@ func (a *Account) Encrypt(passphrase string, scrypt keys.ScryptParams) error { return nil } -// PrivateKey returns private key corresponding to the account. +// PrivateKey returns private key corresponding to the account if it's unlocked. +// Please be very careful when using it, do not copy its contents and do not +// keep a pointer to it unless you absolutely need to. Most of the time you can +// use other methods (PublicKey, ScriptHash, SignHashable) depending on your +// needs and it'll be safer this way. func (a *Account) PrivateKey() *keys.PrivateKey { return a.privateKey } From cbc2299295bbf42dd72d3ee8d170e83a6556ef0a Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 21:03:53 +0300 Subject: [PATCH 10/15] cli/wallet: add warning for dump and export commands --- cli/wallet/wallet.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index b5f467683..932cd0c64 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -173,7 +173,12 @@ func NewCommands() []cli.Command { Name: "dump", Usage: "check and dump an existing NEO wallet", UsageText: "neo-go wallet dump -w wallet [--wallet-config path] [-d]", - Action: dumpWallet, + Description: `Prints the given wallet (via -w option or via wallet configuration file) in JSON + format to the standard output. If -d is given, private keys are unencrypted and + displayed in clear text on the console! Be very careful with this option and + don't use it unless you know what you're doing. +`, + Action: dumpWallet, Flags: []cli.Flag{ walletPathFlag, walletConfigFlag, @@ -198,7 +203,13 @@ func NewCommands() []cli.Command { Name: "export", Usage: "export keys for address", UsageText: "export -w wallet [--wallet-config path] [--decrypt] [
]", - Action: exportKeys, + Description: `Prints the key for the given account to the standard output. It uses NEP-2 + encrypted format by default (the way NEP-6 wallets store it) or WIF format if + -d option is given. In the latter case the key can be displayed in clear text + on the console, so be extremely careful with this option and don't use unless + you really need it and know what you're doing. +`, + Action: exportKeys, Flags: []cli.Flag{ walletPathFlag, walletConfigFlag, From ee5f8b6c21a9fd038105c4b2440eec78cdbfcde4 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 21:42:57 +0300 Subject: [PATCH 11/15] consensus: update dbft, drop marshaling from private key dbft doesn not need this and we must not leak the key in any way. --- go.mod | 4 ++-- go.sum | 7 ++++--- pkg/consensus/crypto.go | 11 ----------- pkg/consensus/crypto_test.go | 9 +-------- 4 files changed, 7 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index da7d338e8..f686cf5be 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/holiman/uint256 v1.2.0 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mr-tron/base58 v1.2.0 - github.com/nspcc-dev/dbft v0.0.0-20220629112714-fd49ca59d354 + github.com/nspcc-dev/dbft v0.0.0-20220902113116-58a5e763e647 github.com/nspcc-dev/go-ordered-json v0.0.0-20220111165707-25110be27d22 github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220809123759-3094d3e0c14b github.com/nspcc-dev/neofs-sdk-go v0.0.0-20220113123743-7f3162110659 @@ -42,7 +42,7 @@ require ( github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect github.com/nspcc-dev/hrw v1.0.9 // indirect github.com/nspcc-dev/neofs-api-go/v2 v2.11.1 // indirect - github.com/nspcc-dev/neofs-crypto v0.3.0 // indirect + github.com/nspcc-dev/neofs-crypto v0.4.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/prometheus/client_model v0.2.0 // indirect github.com/prometheus/common v0.37.0 // indirect diff --git a/go.sum b/go.sum index b75b397ff..7dd037d23 100644 --- a/go.sum +++ b/go.sum @@ -251,8 +251,8 @@ github.com/nspcc-dev/dbft v0.0.0-20191209120240-0d6b7568d9ae/go.mod h1:3FjXOoHmA github.com/nspcc-dev/dbft v0.0.0-20200117124306-478e5cfbf03a/go.mod h1:/YFK+XOxxg0Bfm6P92lY5eDSLYfp06XOdL8KAVgXjVk= github.com/nspcc-dev/dbft v0.0.0-20200219114139-199d286ed6c1/go.mod h1:O0qtn62prQSqizzoagHmuuKoz8QMkU3SzBoKdEvm3aQ= github.com/nspcc-dev/dbft v0.0.0-20210721160347-1b03241391ac/go.mod h1:U8MSnEShH+o5hexfWJdze6uMFJteP0ko7J2frO7Yu1Y= -github.com/nspcc-dev/dbft v0.0.0-20220629112714-fd49ca59d354 h1:/NYQJ9dmeOajNuS0b8x8SMt0b4mEYwUWOpMXuLQ1qwM= -github.com/nspcc-dev/dbft v0.0.0-20220629112714-fd49ca59d354/go.mod h1:U8MSnEShH+o5hexfWJdze6uMFJteP0ko7J2frO7Yu1Y= +github.com/nspcc-dev/dbft v0.0.0-20220902113116-58a5e763e647 h1:handGBjqVzRx7HD6152zsP8ZRxw083zCMbN0IlUaPQk= +github.com/nspcc-dev/dbft v0.0.0-20220902113116-58a5e763e647/go.mod h1:g9xisXmX9NP9MjioaTe862n9SlZTrP+6PVUWLBYOr98= github.com/nspcc-dev/go-ordered-json v0.0.0-20210915112629-e1b6cce73d02/go.mod h1:79bEUDEviBHJMFV6Iq6in57FEOCMcRhfQnfaf0ETA5U= github.com/nspcc-dev/go-ordered-json v0.0.0-20220111165707-25110be27d22 h1:n4ZaFCKt1pQJd7PXoMJabZWK9ejjbLOVrkl/lOUmshg= github.com/nspcc-dev/go-ordered-json v0.0.0-20220111165707-25110be27d22/go.mod h1:79bEUDEviBHJMFV6Iq6in57FEOCMcRhfQnfaf0ETA5U= @@ -267,8 +267,9 @@ github.com/nspcc-dev/neofs-api-go/v2 v2.11.1 h1:SVqc523pZsSaS9vnPS1mm3VV6b6xY0gv github.com/nspcc-dev/neofs-api-go/v2 v2.11.1/go.mod h1:oS8dycEh8PPf2Jjp6+8dlwWyEv2Dy77h/XhhcdxYEFs= github.com/nspcc-dev/neofs-crypto v0.2.0/go.mod h1:F/96fUzPM3wR+UGsPi3faVNmFlA9KAEAUQR7dMxZmNA= github.com/nspcc-dev/neofs-crypto v0.2.3/go.mod h1:8w16GEJbH6791ktVqHN9YRNH3s9BEEKYxGhlFnp0cDw= -github.com/nspcc-dev/neofs-crypto v0.3.0 h1:zlr3pgoxuzrmGCxc5W8dGVfA9Rro8diFvVnBg0L4ifM= github.com/nspcc-dev/neofs-crypto v0.3.0/go.mod h1:8w16GEJbH6791ktVqHN9YRNH3s9BEEKYxGhlFnp0cDw= +github.com/nspcc-dev/neofs-crypto v0.4.0 h1:5LlrUAM5O0k1+sH/sktBtrgfWtq1pgpDs09fZo+KYi4= +github.com/nspcc-dev/neofs-crypto v0.4.0/go.mod h1:6XJ8kbXgOfevbI2WMruOtI+qUJXNwSGM/E9eClXxPHs= github.com/nspcc-dev/neofs-sdk-go v0.0.0-20211201182451-a5b61c4f6477/go.mod h1:dfMtQWmBHYpl9Dez23TGtIUKiFvCIxUZq/CkSIhEpz4= github.com/nspcc-dev/neofs-sdk-go v0.0.0-20220113123743-7f3162110659 h1:rpMCoRa7expLc9gMiOP724gz6YSykZzmMALR/CmiwnU= github.com/nspcc-dev/neofs-sdk-go v0.0.0-20220113123743-7f3162110659/go.mod h1:/jay1lr3w7NQd/VDBkEhkJmDmyPNsu4W+QV2obsUV40= diff --git a/pkg/consensus/crypto.go b/pkg/consensus/crypto.go index 0c6cc69b8..702930f10 100644 --- a/pkg/consensus/crypto.go +++ b/pkg/consensus/crypto.go @@ -13,17 +13,6 @@ type privateKey struct { *keys.PrivateKey } -// MarshalBinary implements the encoding.BinaryMarshaler interface. -func (p privateKey) MarshalBinary() (data []byte, err error) { - return p.PrivateKey.Bytes(), nil -} - -// UnmarshalBinary implements the encoding.BinaryUnmarshaler interface. -func (p *privateKey) UnmarshalBinary(data []byte) (err error) { - p.PrivateKey, err = keys.NewPrivateKeyFromBytes(data) - return -} - // Sign implements the dbft's crypto.PrivateKey interface. func (p *privateKey) Sign(data []byte) ([]byte, error) { return p.PrivateKey.Sign(data), nil diff --git a/pkg/consensus/crypto_test.go b/pkg/consensus/crypto_test.go index 2f6bd10ac..f21fe5df9 100644 --- a/pkg/consensus/crypto_test.go +++ b/pkg/consensus/crypto_test.go @@ -12,19 +12,12 @@ func TestCrypt(t *testing.T) { require.NoError(t, err) priv := privateKey{key} - data, err := priv.MarshalBinary() - require.NoError(t, err) key1, err := keys.NewPrivateKey() require.NoError(t, err) - priv1 := privateKey{key1} - require.NotEqual(t, priv, priv1) - require.NoError(t, priv1.UnmarshalBinary(data)) - require.Equal(t, priv, priv1) - pub := publicKey{key.PublicKey()} - data, err = pub.MarshalBinary() + data, err := pub.MarshalBinary() require.NoError(t, err) pub1 := publicKey{key1.PublicKey()} From 58dc8d0c9b9b6a880a3ee46b51e49d815f770b8d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 21:44:49 +0300 Subject: [PATCH 12/15] *: always close the wallet after use Fix #2631. --- cli/smartcontract/manifest.go | 3 ++- cli/smartcontract/smart_contract.go | 2 ++ cli/wallet/multisig.go | 1 + cli/wallet/nep17.go | 6 ++++++ cli/wallet/validator.go | 1 + cli/wallet/wallet.go | 12 ++++++++++++ pkg/consensus/consensus.go | 1 + pkg/services/notary/notary.go | 1 + pkg/services/oracle/oracle.go | 1 + pkg/services/stateroot/validators.go | 1 + pkg/smartcontract/doc_test.go | 1 + 11 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cli/smartcontract/manifest.go b/cli/smartcontract/manifest.go index a20325208..3964ae1a9 100644 --- a/cli/smartcontract/manifest.go +++ b/cli/smartcontract/manifest.go @@ -37,10 +37,11 @@ func manifestAddGroup(ctx *cli.Context) error { h := state.CreateContractHash(sender, nf.Checksum, m.Name) - gAcc, _, err := getAccFromContext(ctx) + gAcc, w, err := getAccFromContext(ctx) if err != nil { return cli.NewExitError(fmt.Errorf("can't get account to sign group with: %w", err), 1) } + defer w.Close() var found bool diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index fd39fba8f..009cf94e0 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -650,6 +650,7 @@ func invokeInternal(ctx *cli.Context, signAndPush bool) error { if err != nil { return cli.NewExitError(err, 1) } + defer w.Close() } _, err = invokeWithArgs(ctx, acc, w, script, operation, params, cosigners) @@ -949,6 +950,7 @@ func contractDeploy(ctx *cli.Context) error { if err != nil { return cli.NewExitError(fmt.Errorf("can't get sender address: %w", err), 1) } + defer w.Close() cosigners, sgnErr := cmdargs.GetSignersFromContext(ctx, signOffset) if sgnErr != nil { diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index 8b691fd43..2065af51f 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -25,6 +25,7 @@ func signStoredTransaction(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() pc, err := paramcontext.Read(ctx.String("in")) if err != nil { diff --git a/cli/wallet/nep17.go b/cli/wallet/nep17.go index 94077b9ab..6a778cfa3 100644 --- a/cli/wallet/nep17.go +++ b/cli/wallet/nep17.go @@ -222,6 +222,7 @@ func getNEPBalance(ctx *cli.Context, standard string, accHandler func(*cli.Conte if err != nil { return cli.NewExitError(fmt.Errorf("bad wallet: %w", err), 1) } + defer wall.Close() addrFlag := ctx.Generic("address").(*flags.Address) if addrFlag.IsSet { @@ -387,6 +388,7 @@ func importNEPToken(ctx *cli.Context, standard string) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() tokenHashFlag := ctx.Generic("token").(*flags.Address) if !tokenHashFlag.IsSet { @@ -455,6 +457,7 @@ func printNEPInfo(ctx *cli.Context, standard string) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() if name := ctx.String("token"); name != "" { token, err := getMatchingToken(ctx, wall, name, standard) @@ -490,6 +493,7 @@ func removeNEPToken(ctx *cli.Context, standard string) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() token, err := getMatchingToken(ctx, wall, ctx.String("token"), standard) if err != nil { @@ -513,6 +517,7 @@ func multiTransferNEP17(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() fromFlag := ctx.Generic("from").(*flags.Address) from, err := getDefaultAddress(fromFlag, wall) @@ -608,6 +613,7 @@ func transferNEP(ctx *cli.Context, standard string) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() fromFlag := ctx.Generic("from").(*flags.Address) from, err := getDefaultAddress(fromFlag, wall) diff --git a/cli/wallet/validator.go b/cli/wallet/validator.go index ef4726e27..2bc246298 100644 --- a/cli/wallet/validator.go +++ b/cli/wallet/validator.go @@ -94,6 +94,7 @@ func handleNeoAction(ctx *cli.Context, mkTx func(*neo.Contract, util.Uint160, *w if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() addrFlag := ctx.Generic("address").(*flags.Address) if !addrFlag.IsSet { diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index 932cd0c64..54751fb84 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -347,6 +347,7 @@ func claimGas(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() addrFlag := ctx.Generic("address").(*flags.Address) if !addrFlag.IsSet { @@ -388,6 +389,7 @@ func changePassword(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() if len(wall.Accounts) == 0 { return cli.NewExitError("wallet has no accounts", 1) } @@ -483,6 +485,7 @@ func addAccount(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() if err := createAccount(wall, pass); err != nil { return cli.NewExitError(err, 1) @@ -496,6 +499,7 @@ func exportKeys(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() var addr string @@ -557,6 +561,7 @@ func importMultisig(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() m := ctx.Int("min") if ctx.NArg() < m { @@ -600,6 +605,7 @@ func importDeployed(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() rawHash := ctx.Generic("contract").(*flags.Address) if !rawHash.IsSet { @@ -656,6 +662,7 @@ func importWallet(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt) if err != nil { @@ -688,6 +695,7 @@ func removeAccount(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() addr := ctx.Generic("address").(*flags.Address) if !addr.IsSet { @@ -734,6 +742,7 @@ func dumpWallet(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() if ctx.Bool("decrypt") { if pass == nil { password, err := input.ReadPassword(EnterPasswordPrompt) @@ -762,6 +771,7 @@ func dumpKeys(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() accounts := wall.Accounts addrFlag := ctx.Generic("address").(*flags.Address) @@ -812,6 +822,7 @@ func stripKeys(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() if !ctx.Bool("force") { fmt.Fprintln(ctx.App.Writer, "All private keys for all accounts will be removed from the wallet. This action is irreversible.") if ok := askForConsent(ctx.App.Writer); !ok { @@ -861,6 +872,7 @@ func createWallet(ctx *cli.Context) error { if err := createAccount(wall, pass); err != nil { return cli.NewExitError(err, 1) } + defer wall.Close() } fmtPrintWallet(ctx.App.Writer, wall) diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index 7e8523ebd..1c0fc2d23 100644 --- a/pkg/consensus/consensus.go +++ b/pkg/consensus/consensus.go @@ -276,6 +276,7 @@ func (s *service) Shutdown() { s.log.Info("stopping consensus service") close(s.quit) <-s.finished + s.wallet.Close() } } diff --git a/pkg/services/notary/notary.go b/pkg/services/notary/notary.go index f496902e9..c626fb854 100644 --- a/pkg/services/notary/notary.go +++ b/pkg/services/notary/notary.go @@ -222,6 +222,7 @@ func (n *Notary) Shutdown() { n.Config.Log.Info("stopping notary service") close(n.stopCh) <-n.done + n.wallet.Close() } // OnNewRequest is a callback method which is called after a new notary request is added to the notary request pool. diff --git a/pkg/services/oracle/oracle.go b/pkg/services/oracle/oracle.go index d765db731..6dbb8f692 100644 --- a/pkg/services/oracle/oracle.go +++ b/pkg/services/oracle/oracle.go @@ -191,6 +191,7 @@ func (o *Oracle) Shutdown() { close(o.close) o.ResponseHandler.Shutdown() <-o.done + o.wallet.Close() } // Start runs the oracle service in a separate goroutine. diff --git a/pkg/services/stateroot/validators.go b/pkg/services/stateroot/validators.go index fc1bd1db1..02ffe4343 100644 --- a/pkg/services/stateroot/validators.go +++ b/pkg/services/stateroot/validators.go @@ -74,6 +74,7 @@ func (s *service) Shutdown() { s.log.Info("stopping state validation service") close(s.stopCh) <-s.done + s.wallet.Close() } func (s *service) signAndSend(r *state.MPTRoot) error { diff --git a/pkg/smartcontract/doc_test.go b/pkg/smartcontract/doc_test.go index 4dfdda4ca..a74e411f8 100644 --- a/pkg/smartcontract/doc_test.go +++ b/pkg/smartcontract/doc_test.go @@ -33,6 +33,7 @@ func ExampleBuilder() { b.Reset() // Copy the old script above if you need it! w, _ := wallet.NewWalletFromFile("somewhere") + defer w.Close() // Assuming there is one Account inside a, _ := actor.NewSimple(c, w.Accounts[0]) from := w.Accounts[0].Contract.ScriptHash() // Assuming Contract is present. From 74bf4a8e3ffd35d7c99df2d552a3def9375c7a4e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 22:05:34 +0300 Subject: [PATCH 13/15] slice: add Clean microfunction To be used for various cleaning purposes, one line is better than three lines. --- pkg/util/slice/array.go | 7 +++++++ pkg/util/slice/array_test.go | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/pkg/util/slice/array.go b/pkg/util/slice/array.go index cfa759ccf..d6e13f9f9 100644 --- a/pkg/util/slice/array.go +++ b/pkg/util/slice/array.go @@ -28,3 +28,10 @@ func Copy(b []byte) []byte { copy(d, b) return d } + +// Clean wipes the data in b by filling it with zeros. +func Clean(b []byte) { + for i := range b { + b[i] = 0 + } +} diff --git a/pkg/util/slice/array_test.go b/pkg/util/slice/array_test.go index 535f634fc..a5b8f54d6 100644 --- a/pkg/util/slice/array_test.go +++ b/pkg/util/slice/array_test.go @@ -49,3 +49,11 @@ func TestCopyReverse(t *testing.T) { } } } + +func TestClean(t *testing.T) { + for _, tc := range testCases[1:] { // Empty one will be equal. + cp := Copy(tc.arr) + Clean(cp) + require.NotEqual(t, tc.arr, cp) + } +} From 3c722a94989b3537aeca241efafbd39541b47a1e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 22:06:18 +0300 Subject: [PATCH 14/15] keys: clean temporary data during key imports Don't leak anything this way. --- pkg/crypto/keys/nep2.go | 11 ++++++++++- pkg/crypto/keys/private_key.go | 2 ++ pkg/crypto/keys/wif.go | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/crypto/keys/nep2.go b/pkg/crypto/keys/nep2.go index abfa9277d..dd4ab206d 100644 --- a/pkg/crypto/keys/nep2.go +++ b/pkg/crypto/keys/nep2.go @@ -7,6 +7,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/encoding/base58" + "github.com/nspcc-dev/neo-go/pkg/util/slice" "golang.org/x/crypto/scrypt" "golang.org/x/text/unicode/norm" ) @@ -52,10 +53,15 @@ func NEP2Encrypt(priv *PrivateKey, passphrase string, params ScryptParams) (s st if err != nil { return s, err } + defer slice.Clean(derivedKey) derivedKey1 := derivedKey[:32] derivedKey2 := derivedKey[32:] - xr := xor(priv.Bytes(), derivedKey1) + + privBytes := priv.Bytes() + defer slice.Clean(privBytes) + xr := xor(privBytes, derivedKey1) + defer slice.Clean(xr) encrypted, err := aesEncrypt(xr, derivedKey2) if err != nil { @@ -93,6 +99,7 @@ func NEP2Decrypt(key, passphrase string, params ScryptParams) (*PrivateKey, erro if err != nil { return nil, err } + defer slice.Clean(derivedKey) derivedKey1 := derivedKey[:32] derivedKey2 := derivedKey[32:] @@ -102,8 +109,10 @@ func NEP2Decrypt(key, passphrase string, params ScryptParams) (*PrivateKey, erro if err != nil { return nil, err } + defer slice.Clean(decrypted) privBytes := xor(decrypted, derivedKey1) + defer slice.Clean(privBytes) // Rebuild the private key. privKey, err := NewPrivateKeyFromBytes(privBytes) diff --git a/pkg/crypto/keys/private_key.go b/pkg/crypto/keys/private_key.go index 775c8e556..744e49ab1 100644 --- a/pkg/crypto/keys/private_key.go +++ b/pkg/crypto/keys/private_key.go @@ -13,6 +13,7 @@ import ( "github.com/btcsuite/btcd/btcec" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/util/slice" "github.com/nspcc-dev/rfc6979" ) @@ -48,6 +49,7 @@ func NewPrivateKeyFromHex(str string) (*PrivateKey, error) { if err != nil { return nil, err } + defer slice.Clean(b) return NewPrivateKeyFromBytes(b) } diff --git a/pkg/crypto/keys/wif.go b/pkg/crypto/keys/wif.go index 0e4d57b3d..7da78ea8e 100644 --- a/pkg/crypto/keys/wif.go +++ b/pkg/crypto/keys/wif.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/nspcc-dev/neo-go/pkg/encoding/base58" + "github.com/nspcc-dev/neo-go/pkg/util/slice" ) const ( @@ -53,6 +54,7 @@ func WIFDecode(wif string, version byte) (*WIF, error) { if err != nil { return nil, err } + defer slice.Clean(b) if version == 0x00 { version = WIFVersion From eb67145f8157bce8b78293072a6af355b89a66a5 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 1 Sep 2022 22:23:26 +0300 Subject: [PATCH 15/15] keys: check length first, then do things in WIFDecode Otherwise we can easily panic there on bad input. --- pkg/crypto/keys/private_key.go | 4 +++- pkg/crypto/keys/wif.go | 39 +++++++++++++++------------------- pkg/crypto/keys/wif_test.go | 33 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 23 deletions(-) diff --git a/pkg/crypto/keys/private_key.go b/pkg/crypto/keys/private_key.go index 744e49ab1..8facca0d9 100644 --- a/pkg/crypto/keys/private_key.go +++ b/pkg/crypto/keys/private_key.go @@ -110,7 +110,9 @@ func NewPrivateKeyFromWIF(wif string) (*PrivateKey, error) { // Good documentation about this process can be found here: // https://en.bitcoin.it/wiki/Wallet_import_format func (p *PrivateKey) WIF() string { - w, err := WIFEncode(p.Bytes(), WIFVersion, true) + pb := p.Bytes() + defer slice.Clean(pb) + w, err := WIFEncode(pb, WIFVersion, true) // The only way WIFEncode() can fail is if we're to give it a key of // wrong size, but we have a proper key here, aren't we? if err != nil { diff --git a/pkg/crypto/keys/wif.go b/pkg/crypto/keys/wif.go index 7da78ea8e..1ec908cdd 100644 --- a/pkg/crypto/keys/wif.go +++ b/pkg/crypto/keys/wif.go @@ -59,36 +59,31 @@ func WIFDecode(wif string, version byte) (*WIF, error) { if version == 0x00 { version = WIFVersion } + w := &WIF{ + Version: version, + S: wif, + } + switch len(b) { + case 33: // OK, uncompressed public key. + case 34: // OK, compressed public key. + // Check the compression flag. + if b[33] != 0x01 { + return nil, fmt.Errorf("invalid compression flag %d expecting %d", b[33], 0x01) + } + w.Compressed = true + default: + return nil, fmt.Errorf("invalid WIF length %d, expecting 33 or 34", len(b)) + } + if b[0] != version { return nil, fmt.Errorf("invalid WIF version got %d, expected %d", b[0], version) } // Derive the PrivateKey. - privKey, err := NewPrivateKeyFromBytes(b[1:33]) + w.PrivateKey, err = NewPrivateKeyFromBytes(b[1:33]) if err != nil { return nil, err } - w := &WIF{ - Version: version, - PrivateKey: privKey, - S: wif, - } - // This is an uncompressed WIF. - if len(b) == 33 { - w.Compressed = false - return w, nil - } - - if len(b) != 34 { - return nil, fmt.Errorf("invalid WIF length: %d expecting 34", len(b)) - } - - // Check the compression flag. - if b[33] != 0x01 { - return nil, fmt.Errorf("invalid compression flag %d expecting %d", b[34], 0x01) - } - - w.Compressed = true return w, nil } diff --git a/pkg/crypto/keys/wif_test.go b/pkg/crypto/keys/wif_test.go index 6fae58167..f29269e72 100644 --- a/pkg/crypto/keys/wif_test.go +++ b/pkg/crypto/keys/wif_test.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "testing" + "github.com/nspcc-dev/neo-go/pkg/encoding/base58" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -65,3 +66,35 @@ func TestWIFEncodeDecode(t *testing.T) { _, err := WIFEncode(wifInv, 0, true) require.Error(t, err) } + +func TestBadWIFDecode(t *testing.T) { + _, err := WIFDecode("garbage", 0) + require.Error(t, err) + + s := base58.CheckEncode([]byte{}) + _, err = WIFDecode(s, 0) + require.Error(t, err) + + uncompr := make([]byte, 33) + compr := make([]byte, 34) + + s = base58.CheckEncode(compr) + _, err = WIFDecode(s, 0) + require.Error(t, err) + + s = base58.CheckEncode(uncompr) + _, err = WIFDecode(s, 0) + require.Error(t, err) + + compr[33] = 1 + compr[0] = WIFVersion + uncompr[0] = WIFVersion + + s = base58.CheckEncode(compr) + _, err = WIFDecode(s, 0) + require.NoError(t, err) + + s = base58.CheckEncode(uncompr) + _, err = WIFDecode(s, 0) + require.NoError(t, err) +}