From 2f8896f7a166605e8bcad945ae04c1976703c99f Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 23 Aug 2022 22:00:23 +0300 Subject: [PATCH 1/8] rpcclient: add notary subpackage with the notary contract wrapper --- pkg/rpcclient/native.go | 3 + pkg/rpcclient/notary/contract.go | 227 ++++++++++++++++++++++++++ pkg/rpcclient/notary/contract_test.go | 199 ++++++++++++++++++++++ pkg/services/rpcsrv/client_test.go | 91 ++++++++++- 4 files changed, 519 insertions(+), 1 deletion(-) create mode 100644 pkg/rpcclient/notary/contract.go create mode 100644 pkg/rpcclient/notary/contract_test.go diff --git a/pkg/rpcclient/native.go b/pkg/rpcclient/native.go index 0b6526c44..3d41265fd 100644 --- a/pkg/rpcclient/native.go +++ b/pkg/rpcclient/native.go @@ -150,6 +150,9 @@ func (c *Client) NNSUnpackedGetAllRecords(nnsHash util.Uint160, name string) ([] // GetNotaryServiceFeePerKey returns a reward per notary request key for the designated // notary nodes. It doesn't cache the result. +// +// Deprecated: please use the Notary contract wrapper from the notary subpackage. This +// method will be removed in future versions. func (c *Client) GetNotaryServiceFeePerKey() (int64, error) { notaryHash, err := c.GetNativeContractHash(nativenames.Notary) if err != nil { diff --git a/pkg/rpcclient/notary/contract.go b/pkg/rpcclient/notary/contract.go new file mode 100644 index 000000000..aa216cbd4 --- /dev/null +++ b/pkg/rpcclient/notary/contract.go @@ -0,0 +1,227 @@ +/* +Package notary provides an RPC-based wrapper for the Notary subsystem. + +It provides both regular ContractReader/Contract interfaces for the notary +contract and notary-specific functions and interfaces to simplify creation of +notary requests. +*/ +package notary + +import ( + "math" + "math/big" + + "github.com/nspcc-dev/neo-go/pkg/core/native/nativenames" + "github.com/nspcc-dev/neo-go/pkg/core/state" + "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "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" + "github.com/nspcc-dev/neo-go/pkg/util" +) + +const ( + setMaxNVBDeltaMethod = "setMaxNotValidBeforeDelta" + setFeePKMethod = "setNotaryServiceFeePerKey" +) + +// ContractInvoker is used by ContractReader to perform read-only calls. +type ContractInvoker interface { + Call(contract util.Uint160, operation string, params ...interface{}) (*result.Invoke, error) +} + +// ContractActor is used by Contract to create and send transactions. +type ContractActor interface { + ContractInvoker + + MakeCall(contract util.Uint160, method string, params ...interface{}) (*transaction.Transaction, error) + MakeRun(script []byte) (*transaction.Transaction, error) + MakeUnsignedCall(contract util.Uint160, method string, attrs []transaction.Attribute, params ...interface{}) (*transaction.Transaction, error) + MakeUnsignedRun(script []byte, attrs []transaction.Attribute) (*transaction.Transaction, error) + SendCall(contract util.Uint160, method string, params ...interface{}) (util.Uint256, uint32, error) + SendRun(script []byte) (util.Uint256, uint32, error) +} + +// ContractReader represents safe (read-only) methods of Notary. It can be +// used to query various data, but `verify` method is not exposed there because +// it can't be successful in standalone invocation (missing transaction with the +// NotaryAssisted attribute and its signature). +type ContractReader struct { + invoker ContractInvoker +} + +// Contract provides full Notary interface, both safe and state-changing methods. +// The only method omitted is onNEP17Payment which can only be called +// successfully from the GASToken native contract. +type Contract struct { + ContractReader + + actor ContractActor +} + +// Hash stores the hash of the native Notary contract. +var Hash = state.CreateNativeContractHash(nativenames.Notary) + +// NewReader creates an instance of ContractReader to get data from the Notary +// contract. +func NewReader(invoker ContractInvoker) *ContractReader { + return &ContractReader{invoker} +} + +// New creates an instance of Contract to perform state-changing actions in the +// Notary contract. +func New(actor ContractActor) *Contract { + return &Contract{*NewReader(actor), actor} +} + +// BalanceOf returns the locked GAS balance for the given account. +func (c *ContractReader) BalanceOf(account util.Uint160) (*big.Int, error) { + return unwrap.BigInt(c.invoker.Call(Hash, "balanceOf", account)) +} + +// ExpirationOf returns the index of the block when the GAS deposit for the given +// account will expire. +func (c *ContractReader) ExpirationOf(account util.Uint160) (uint32, error) { + res, err := c.invoker.Call(Hash, "expirationOf", account) + ret, err := unwrap.LimitedInt64(res, err, 0, math.MaxUint32) + return uint32(ret), err +} + +// GetMaxNotValidBeforeDelta returns the maximum NotValidBefore attribute delta +// that can be used in notary-assisted transactions. +func (c *ContractReader) GetMaxNotValidBeforeDelta() (uint32, error) { + res, err := c.invoker.Call(Hash, "getMaxNotValidBeforeDelta") + ret, err := unwrap.LimitedInt64(res, err, 0, math.MaxUint32) + return uint32(ret), err +} + +// GetNotaryServiceFeePerKey returns the per-key fee amount paid by transactions +// for the NotaryAssisted attribute. +func (c *ContractReader) GetNotaryServiceFeePerKey() (int64, error) { + return unwrap.Int64(c.invoker.Call(Hash, "getNotaryServiceFeePerKey")) +} + +// LockDepositUntil creates and sends a transaction that extends the deposit lock +// time for the given account. The return result from the "lockDepositUntil" +// method is checked to be true, so transaction fails (with FAULT state) if not +// successful. The returned values are transaction hash, its ValidUntilBlock +// value and an error if any. +func (c *Contract) LockDepositUntil(account util.Uint160, index uint32) (util.Uint256, uint32, error) { + return c.actor.SendRun(lockScript(account, index)) +} + +// LockDepositUntilTransaction creates a transaction that extends the deposit lock +// time for the given account. The return result from the "lockDepositUntil" +// method is checked to be true, so transaction fails (with FAULT state) if not +// successful. The returned values are transaction hash, its ValidUntilBlock +// value and an error if any. The transaction is signed, but not sent to the +// network, instead it's returned to the caller. +func (c *Contract) LockDepositUntilTransaction(account util.Uint160, index uint32) (*transaction.Transaction, error) { + return c.actor.MakeRun(lockScript(account, index)) +} + +// LockDepositUntilUnsigned creates a transaction that extends the deposit lock +// time for the given account. The return result from the "lockDepositUntil" +// method is checked to be true, so transaction fails (with FAULT state) if not +// successful. The returned values are transaction hash, its ValidUntilBlock +// value and an error if any. The transaction is not signed and just returned to +// the caller. +func (c *Contract) LockDepositUntilUnsigned(account util.Uint160, index uint32) (*transaction.Transaction, error) { + return c.actor.MakeUnsignedRun(lockScript(account, index), nil) +} + +func lockScript(account util.Uint160, index uint32) []byte { + // We know parameters exactly (unlike with nep17.Transfer), so this can't fail. + script, _ := smartcontract.CreateCallWithAssertScript(Hash, "lockDepositUntil", account.BytesBE(), int64(index)) + return script +} + +// SetMaxNotValidBeforeDelta creates and sends a transaction that sets the new +// maximum NotValidBefore attribute value delta that can be used in +// notary-assisted transactions. The action is successful when transaction +// ends in HALT state. Notice that this setting can be changed only by the +// network's committee, so use an appropriate Actor. The returned values are +// transaction hash, its ValidUntilBlock value and an error if any. +func (c *Contract) SetMaxNotValidBeforeDelta(blocks uint32) (util.Uint256, uint32, error) { + return c.actor.SendCall(Hash, setMaxNVBDeltaMethod, blocks) +} + +// SetMaxNotValidBeforeDeltaTransaction creates a transaction that sets the new +// maximum NotValidBefore attribute value delta that can be used in +// notary-assisted transactions. The action is successful when transaction +// ends in HALT state. Notice that this setting can be changed only by the +// network's committee, so use an appropriate Actor. The transaction is signed, +// but not sent to the network, instead it's returned to the caller. +func (c *Contract) SetMaxNotValidBeforeDeltaTransaction(blocks uint32) (*transaction.Transaction, error) { + return c.actor.MakeCall(Hash, setMaxNVBDeltaMethod, blocks) +} + +// SetMaxNotValidBeforeDeltaUnsigned creates a transaction that sets the new +// maximum NotValidBefore attribute value delta that can be used in +// notary-assisted transactions. The action is successful when transaction +// ends in HALT state. Notice that this setting can be changed only by the +// network's committee, so use an appropriate Actor. The transaction is not +// signed and just returned to the caller. +func (c *Contract) SetMaxNotValidBeforeDeltaUnsigned(blocks uint32) (*transaction.Transaction, error) { + return c.actor.MakeUnsignedCall(Hash, setMaxNVBDeltaMethod, nil, blocks) +} + +// SetNotaryServiceFeePerKey creates and sends a transaction that sets the new +// per-key fee value paid for using the notary service. The action is successful +// when transaction ends in HALT state. Notice that this setting can be changed +// only by the network's committee, so use an appropriate Actor. The returned +// values are transaction hash, its ValidUntilBlock value and an error if any. +func (c *Contract) SetNotaryServiceFeePerKey(fee int64) (util.Uint256, uint32, error) { + return c.actor.SendCall(Hash, setFeePKMethod, fee) +} + +// SetNotaryServiceFeePerKeyTransaction creates a transaction that sets the new +// per-key fee value paid for using the notary service. The action is successful +// when transaction ends in HALT state. Notice that this setting can be changed +// only by the network's committee, so use an appropriate Actor. The transaction +// is signed, but not sent to the network, instead it's returned to the caller. +func (c *Contract) SetNotaryServiceFeePerKeyTransaction(fee int64) (*transaction.Transaction, error) { + return c.actor.MakeCall(Hash, setFeePKMethod, fee) +} + +// SetNotaryServiceFeePerKeyUnsigned creates a transaction that sets the new +// per-key fee value paid for using the notary service. The action is successful +// when transaction ends in HALT state. Notice that this setting can be changed +// only by the network's committee, so use an appropriate Actor. The transaction +// is not signed and just returned to the caller. +func (c *Contract) SetNotaryServiceFeePerKeyUnsigned(fee int64) (*transaction.Transaction, error) { + return c.actor.MakeUnsignedCall(Hash, setFeePKMethod, nil, fee) +} + +// Withdraw creates and sends a transaction that withdraws the deposit belonging +// to "from" account and sends it to "to" account. The return result from the +// "withdraw" method is checked to be true, so transaction fails (with FAULT +// state) if not successful. The returned values are transaction hash, its +// ValidUntilBlock value and an error if any. +func (c *Contract) Withdraw(from util.Uint160, to util.Uint160) (util.Uint256, uint32, error) { + return c.actor.SendRun(withdrawScript(from, to)) +} + +// WithdrawTransaction creates a transaction that withdraws the deposit belonging +// to "from" account and sends it to "to" account. The return result from the +// "withdraw" method is checked to be true, so transaction fails (with FAULT +// state) if not successful. The transaction is signed, but not sent to the +// network, instead it's returned to the caller. +func (c *Contract) WithdrawTransaction(from util.Uint160, to util.Uint160) (*transaction.Transaction, error) { + return c.actor.MakeRun(withdrawScript(from, to)) +} + +// WithdrawUnsigned creates a transaction that withdraws the deposit belonging +// to "from" account and sends it to "to" account. The return result from the +// "withdraw" method is checked to be true, so transaction fails (with FAULT +// state) if not successful. The transaction is not signed and just returned to +// the caller. +func (c *Contract) WithdrawUnsigned(from util.Uint160, to util.Uint160) (*transaction.Transaction, error) { + return c.actor.MakeUnsignedRun(withdrawScript(from, to), nil) +} + +func withdrawScript(from util.Uint160, to util.Uint160) []byte { + // We know parameters exactly (unlike with nep17.Transfer), so this can't fail. + script, _ := smartcontract.CreateCallWithAssertScript(Hash, "withdraw", from.BytesBE(), to.BytesBE()) + return script +} diff --git a/pkg/rpcclient/notary/contract_test.go b/pkg/rpcclient/notary/contract_test.go new file mode 100644 index 000000000..c8498ffbf --- /dev/null +++ b/pkg/rpcclient/notary/contract_test.go @@ -0,0 +1,199 @@ +package notary + +import ( + "errors" + "math/big" + "testing" + + "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/neorpc/result" + "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/stretchr/testify/require" +) + +type testAct struct { + err error + res *result.Invoke + tx *transaction.Transaction + txh util.Uint256 + vub uint32 +} + +func (t *testAct) Call(contract util.Uint160, operation string, params ...interface{}) (*result.Invoke, error) { + return t.res, t.err +} +func (t *testAct) MakeRun(script []byte) (*transaction.Transaction, error) { + return t.tx, t.err +} +func (t *testAct) MakeUnsignedRun(script []byte, attrs []transaction.Attribute) (*transaction.Transaction, error) { + return t.tx, t.err +} +func (t *testAct) SendRun(script []byte) (util.Uint256, uint32, error) { + return t.txh, t.vub, t.err +} +func (t *testAct) MakeCall(contract util.Uint160, method string, params ...interface{}) (*transaction.Transaction, error) { + return t.tx, t.err +} +func (t *testAct) MakeUnsignedCall(contract util.Uint160, method string, attrs []transaction.Attribute, params ...interface{}) (*transaction.Transaction, error) { + return t.tx, t.err +} +func (t *testAct) SendCall(contract util.Uint160, method string, params ...interface{}) (util.Uint256, uint32, error) { + return t.txh, t.vub, t.err +} + +func TestBalanceOf(t *testing.T) { + ta := &testAct{} + ntr := NewReader(ta) + + ta.err = errors.New("") + _, err := ntr.BalanceOf(util.Uint160{}) + require.Error(t, err) + + ta.err = nil + ta.res = &result.Invoke{ + State: "HALT", + Stack: []stackitem.Item{ + stackitem.Make(42), + }, + } + res, err := ntr.BalanceOf(util.Uint160{}) + require.NoError(t, err) + require.Equal(t, big.NewInt(42), res) +} + +func TestUint32Getters(t *testing.T) { + ta := &testAct{} + ntr := NewReader(ta) + + for name, fun := range map[string]func() (uint32, error){ + "ExpirationOf": func() (uint32, error) { + return ntr.ExpirationOf(util.Uint160{1, 2, 3}) + }, + "GetMaxNotValidBeforeDelta": ntr.GetMaxNotValidBeforeDelta, + } { + t.Run(name, func(t *testing.T) { + ta.err = errors.New("") + _, err := fun() + require.Error(t, err) + + ta.err = nil + ta.res = &result.Invoke{ + State: "HALT", + Stack: []stackitem.Item{ + stackitem.Make(42), + }, + } + res, err := fun() + require.NoError(t, err) + require.Equal(t, uint32(42), res) + + ta.res = &result.Invoke{ + State: "HALT", + Stack: []stackitem.Item{ + stackitem.Make(-1), + }, + } + _, err = fun() + require.Error(t, err) + }) + } +} + +func TestGetNotaryServiceFeePerKey(t *testing.T) { + ta := &testAct{} + ntr := NewReader(ta) + + ta.err = errors.New("") + _, err := ntr.GetNotaryServiceFeePerKey() + require.Error(t, err) + + ta.err = nil + ta.res = &result.Invoke{ + State: "HALT", + Stack: []stackitem.Item{ + stackitem.Make(42), + }, + } + res, err := ntr.GetNotaryServiceFeePerKey() + require.NoError(t, err) + require.Equal(t, int64(42), res) +} + +func TestTxSenders(t *testing.T) { + ta := new(testAct) + ntr := New(ta) + + for name, fun := range map[string]func() (util.Uint256, uint32, error){ + "LockDepositUntil": func() (util.Uint256, uint32, error) { + return ntr.LockDepositUntil(util.Uint160{1, 2, 3}, 100500) + }, + "SetMaxNotValidBeforeDelta": func() (util.Uint256, uint32, error) { + return ntr.SetMaxNotValidBeforeDelta(42) + }, + "SetNotaryServiceFeePerKey": func() (util.Uint256, uint32, error) { + return ntr.SetNotaryServiceFeePerKey(100500) + }, + "Withdraw": func() (util.Uint256, uint32, error) { + return ntr.Withdraw(util.Uint160{1, 2, 3}, util.Uint160{3, 2, 1}) + }, + } { + t.Run(name, func(t *testing.T) { + ta.err = errors.New("") + _, _, err := fun() + require.Error(t, err) + + ta.err = nil + ta.txh = util.Uint256{1, 2, 3} + ta.vub = 42 + h, vub, err := fun() + require.NoError(t, err) + require.Equal(t, ta.txh, h) + require.Equal(t, ta.vub, vub) + }) + } +} + +func TestTxMakers(t *testing.T) { + ta := new(testAct) + ntr := New(ta) + + for name, fun := range map[string]func() (*transaction.Transaction, error){ + "LockDepositUntilTransaction": func() (*transaction.Transaction, error) { + return ntr.LockDepositUntilTransaction(util.Uint160{1, 2, 3}, 100500) + }, + "LockDepositUntilUnsigned": func() (*transaction.Transaction, error) { + return ntr.LockDepositUntilUnsigned(util.Uint160{1, 2, 3}, 100500) + }, + "SetMaxNotValidBeforeDeltaTransaction": func() (*transaction.Transaction, error) { + return ntr.SetMaxNotValidBeforeDeltaTransaction(42) + }, + "SetMaxNotValidBeforeDeltaUnsigned": func() (*transaction.Transaction, error) { + return ntr.SetMaxNotValidBeforeDeltaUnsigned(42) + }, + "SetNotaryServiceFeePerKeyTransaction": func() (*transaction.Transaction, error) { + return ntr.SetNotaryServiceFeePerKeyTransaction(100500) + }, + "SetNotaryServiceFeePerKeyUnsigned": func() (*transaction.Transaction, error) { + return ntr.SetNotaryServiceFeePerKeyUnsigned(100500) + }, + "WithdrawTransaction": func() (*transaction.Transaction, error) { + return ntr.WithdrawTransaction(util.Uint160{1, 2, 3}, util.Uint160{3, 2, 1}) + }, + "WithdrawUnsigned": func() (*transaction.Transaction, error) { + return ntr.WithdrawUnsigned(util.Uint160{1, 2, 3}, util.Uint160{3, 2, 1}) + }, + } { + t.Run(name, func(t *testing.T) { + ta.err = errors.New("") + _, err := fun() + require.Error(t, err) + + ta.err = nil + ta.tx = &transaction.Transaction{Nonce: 100500, ValidUntilBlock: 42} + tx, err := fun() + require.NoError(t, err) + require.Equal(t, ta.tx, tx) + }) + } +} diff --git a/pkg/services/rpcsrv/client_test.go b/pkg/services/rpcsrv/client_test.go index c977681e7..ec3a6b70d 100644 --- a/pkg/services/rpcsrv/client_test.go +++ b/pkg/services/rpcsrv/client_test.go @@ -37,6 +37,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/rpcclient/nep11" "github.com/nspcc-dev/neo-go/pkg/rpcclient/nep17" "github.com/nspcc-dev/neo-go/pkg/rpcclient/nns" + "github.com/nspcc-dev/neo-go/pkg/rpcclient/notary" "github.com/nspcc-dev/neo-go/pkg/rpcclient/oracle" "github.com/nspcc-dev/neo-go/pkg/rpcclient/policy" "github.com/nspcc-dev/neo-go/pkg/rpcclient/rolemgmt" @@ -422,6 +423,94 @@ func TestClientNEOContract(t *testing.T) { require.NoError(t, err) } +func TestClientNotary(t *testing.T) { + chain, rpcSrv, httpSrv := initServerWithInMemoryChain(t) + defer chain.Close() + defer rpcSrv.Shutdown() + + c, err := rpcclient.New(context.Background(), httpSrv.URL, rpcclient.Options{}) + require.NoError(t, err) + require.NoError(t, c.Init()) + + notaReader := notary.NewReader(invoker.New(c, nil)) + + priv0 := testchain.PrivateKeyByID(0) + priv0Hash := priv0.PublicKey().GetScriptHash() + bal, err := notaReader.BalanceOf(priv0Hash) + require.NoError(t, err) + require.Equal(t, big.NewInt(10_0000_0000), bal) + + expir, err := notaReader.ExpirationOf(priv0Hash) + require.NoError(t, err) + require.Equal(t, uint32(1007), expir) + + maxNVBd, err := notaReader.GetMaxNotValidBeforeDelta() + require.NoError(t, err) + require.Equal(t, uint32(140), maxNVBd) + + feePerKey, err := notaReader.GetNotaryServiceFeePerKey() + require.NoError(t, err) + require.Equal(t, int64(1000_0000), feePerKey) + + commAct, err := actor.New(c, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: testchain.CommitteeScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: &wallet.Account{ + Address: testchain.CommitteeAddress(), + Contract: &wallet.Contract{ + Script: testchain.CommitteeVerificationScript(), + }, + }, + }}) + require.NoError(t, err) + notaComm := notary.New(commAct) + + txNVB, err := notaComm.SetMaxNotValidBeforeDeltaUnsigned(210) + require.NoError(t, err) + txFee, err := notaComm.SetNotaryServiceFeePerKeyUnsigned(500_0000) + require.NoError(t, err) + + txNVB.Scripts[0].InvocationScript = testchain.SignCommittee(txNVB) + txFee.Scripts[0].InvocationScript = testchain.SignCommittee(txFee) + bl := testchain.NewBlock(t, chain, 1, 0, txNVB, txFee) + _, err = c.SubmitBlock(*bl) + require.NoError(t, err) + + maxNVBd, err = notaReader.GetMaxNotValidBeforeDelta() + require.NoError(t, err) + require.Equal(t, uint32(210), maxNVBd) + + feePerKey, err = notaReader.GetNotaryServiceFeePerKey() + require.NoError(t, err) + require.Equal(t, int64(500_0000), feePerKey) + + privAct, err := actor.New(c, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: priv0Hash, + Scopes: transaction.CalledByEntry, + }, + Account: wallet.NewAccountFromPrivateKey(priv0), + }}) + require.NoError(t, err) + notaPriv := notary.New(privAct) + + txLock, err := notaPriv.LockDepositUntilTransaction(priv0Hash, 1111) + require.NoError(t, err) + + bl = testchain.NewBlock(t, chain, 1, 0, txLock) + _, err = c.SubmitBlock(*bl) + require.NoError(t, err) + + expir, err = notaReader.ExpirationOf(priv0Hash) + require.NoError(t, err) + require.Equal(t, uint32(1111), expir) + + _, err = notaPriv.WithdrawTransaction(priv0Hash, priv0Hash) + require.Error(t, err) // Can't be withdrawn until 1111. +} + func TestAddNetworkFeeCalculateNetworkFee(t *testing.T) { chain, rpcSrv, httpSrv := initServerWithInMemoryChain(t) defer chain.Close() @@ -1618,7 +1707,7 @@ func TestClient_GetNotaryServiceFeePerKey(t *testing.T) { require.NoError(t, c.Init()) var defaultNotaryServiceFeePerKey int64 = 1000_0000 - actual, err := c.GetNotaryServiceFeePerKey() + actual, err := c.GetNotaryServiceFeePerKey() //nolint:staticcheck // SA1019: c.GetNotaryServiceFeePerKey is deprecated require.NoError(t, err) require.Equal(t, defaultNotaryServiceFeePerKey, actual) } From 7a930a8e1176767bb5e2591e0fc0d997a082e89f Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 24 Aug 2022 15:36:52 +0300 Subject: [PATCH 2/8] wallet: don't fail in SignTx when no contract provided Unfortunately valid NEP-6 can have no contract inside of account, so this should be accounted for. --- pkg/wallet/account.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 7b1619707..fff9dde7b 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -84,6 +84,9 @@ func NewAccount() (*Account, error) { // SignTx signs transaction t and updates it's Witnesses. func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { + if a.Contract == nil { + return errors.New("account has no contract") + } if len(a.Contract.Parameters) == 0 { if len(t.Signers) != len(t.Scripts) { // Sequential signing vs. existing scripts. t.Scripts = append(t.Scripts, transaction.Witness{}) From 54c5fd61df0a553a659b1cbeb0a52d14893456cc Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 24 Aug 2022 17:09:57 +0300 Subject: [PATCH 3/8] wallet: make SignTx more precise and accurate * each account must have an appropriate signer, if there is no signer for this account in the tx it's an error * we can only safely append to Scripts when account belongs to the next signer (we don't have appropriate verification scripts for other signers) * when contract has one parameter, the signature shouldn't be appended to other data I think these rules allow to handle more cases and do that safer. We have more complex scenarios though, like non-signature parameters or mixed-parameter invocation scripts, but that's out of scope for now. --- pkg/wallet/account.go | 45 +++++++++++++++++------- pkg/wallet/account_test.go | 71 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index fff9dde7b..335beba72 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -84,13 +84,38 @@ func NewAccount() (*Account, error) { // SignTx signs transaction t and updates it's Witnesses. func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { + var ( + haveAcc bool + pos int + accHash util.Uint160 + err error + ) if a.Contract == nil { return errors.New("account has no contract") } - if len(a.Contract.Parameters) == 0 { - if len(t.Signers) != len(t.Scripts) { // Sequential signing vs. existing scripts. - t.Scripts = append(t.Scripts, transaction.Witness{}) + accHash, err = address.StringToUint160(a.Address) + if err != nil { + return err + } + for i := range t.Signers { + if t.Signers[i].Account.Equals(accHash) { + haveAcc = true + pos = i + break } + } + if !haveAcc { + return errors.New("transaction is not signed by this account") + } + if len(t.Scripts) < pos { + return errors.New("transaction is not yet signed by the previous signer") + } + if len(t.Scripts) == pos { + t.Scripts = append(t.Scripts, transaction.Witness{ + VerificationScript: a.Contract.Script, // Can be nil for deployed contract. + }) + } + if len(a.Contract.Parameters) == 0 { return nil } if a.privateKey == nil { @@ -98,18 +123,12 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { } sign := a.privateKey.SignHashable(uint32(net), t) - verif := a.GetVerificationScript() invoc := append([]byte{byte(opcode.PUSHDATA1), 64}, sign...) - for i := range t.Scripts { - if bytes.Equal(t.Scripts[i].VerificationScript, verif) { - t.Scripts[i].InvocationScript = append(t.Scripts[i].InvocationScript, invoc...) - return nil - } + if len(a.Contract.Parameters) == 1 { + t.Scripts[pos].InvocationScript = invoc + } else { + t.Scripts[pos].InvocationScript = append(t.Scripts[pos].InvocationScript, invoc...) } - t.Scripts = append(t.Scripts, transaction.Witness{ - InvocationScript: invoc, - VerificationScript: verif, - }) return nil } diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 23333d066..1e328a857 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -6,8 +6,10 @@ import ( "testing" "github.com/nspcc-dev/neo-go/internal/keytestcases" + "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/smartcontract" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -81,6 +83,75 @@ func TestContract_MarshalJSON(t *testing.T) { require.Error(t, json.Unmarshal(data, &c)) } +func TestContractSignTx(t *testing.T) { + acc, err := NewAccount() + require.NoError(t, err) + + accNoContr := *acc + accNoContr.Contract = nil + tx := &transaction.Transaction{ + Script: []byte{1, 2, 3}, + Signers: []transaction.Signer{{ + Account: acc.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }}, + } + require.Error(t, accNoContr.SignTx(0, tx)) + + acc2, err := NewAccount() + require.NoError(t, err) + + require.Error(t, acc2.SignTx(0, tx)) + + pubs := keys.PublicKeys{acc.privateKey.PublicKey(), acc2.privateKey.PublicKey()} + multiS, err := smartcontract.CreateDefaultMultiSigRedeemScript(pubs) + require.NoError(t, err) + multiAcc := NewAccountFromPrivateKey(acc.privateKey) + require.NoError(t, multiAcc.ConvertMultisig(2, pubs)) + multiAcc2 := NewAccountFromPrivateKey(acc2.privateKey) + require.NoError(t, multiAcc2.ConvertMultisig(2, pubs)) + + tx = &transaction.Transaction{ + Script: []byte{1, 2, 3}, + Signers: []transaction.Signer{{ + Account: acc2.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, { + Account: acc.Contract.ScriptHash(), + Scopes: transaction.None, + }, { + Account: hash.Hash160(multiS), + Scopes: transaction.None, + }}, + } + require.Error(t, acc.SignTx(0, tx)) // Can't append, no witness for acc2. + + require.NoError(t, acc2.SignTx(0, tx)) // Append script for acc2. + require.Equal(t, 1, len(tx.Scripts)) + require.Equal(t, 66, len(tx.Scripts[0].InvocationScript)) + + require.NoError(t, acc2.SignTx(0, tx)) // Sign again, effectively a no-op. + require.Equal(t, 1, len(tx.Scripts)) + require.Equal(t, 66, len(tx.Scripts[0].InvocationScript)) + + acc2.privateKey = nil + require.Error(t, acc2.SignTx(0, tx)) // No private key. + + tx.Scripts = append(tx.Scripts, transaction.Witness{ + VerificationScript: acc.Contract.Script, + }) + require.NoError(t, acc.SignTx(0, tx)) // Add invocation script for existing witness. + require.Equal(t, 66, len(tx.Scripts[1].InvocationScript)) + + require.NoError(t, multiAcc.SignTx(0, tx)) + require.Equal(t, 3, len(tx.Scripts)) + require.Equal(t, 66, len(tx.Scripts[2].InvocationScript)) + + require.NoError(t, multiAcc2.SignTx(0, tx)) // Append to existing script. + require.Equal(t, 3, len(tx.Scripts)) + require.Equal(t, 132, len(tx.Scripts[2].InvocationScript)) +} + func TestContract_ScriptHash(t *testing.T) { script := []byte{0, 1, 2, 3} c := &Contract{Script: script} From 8b132cba0c7638756074303e56856889401e64b6 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 24 Aug 2022 17:40:14 +0300 Subject: [PATCH 4/8] wallet: respect user-locked accounts, don't sign with them NEP-6 has a notion of locked acccounts and SignTx must respect this user's choice. For some reason this setting was inappropriately used by our RPC client tests (probably a different kind of lock was meant). --- pkg/services/rpcsrv/client_test.go | 2 -- pkg/wallet/account.go | 5 ++++- pkg/wallet/account_test.go | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/services/rpcsrv/client_test.go b/pkg/services/rpcsrv/client_test.go index ec3a6b70d..5315c8f16 100644 --- a/pkg/services/rpcsrv/client_test.go +++ b/pkg/services/rpcsrv/client_test.go @@ -850,7 +850,6 @@ func TestSignAndPushInvocationTx(t *testing.T) { Parameters: []wallet.ContractParam{}, Deployed: true, }, - Locked: true, Default: false, } @@ -866,7 +865,6 @@ func TestSignAndPushInvocationTx(t *testing.T) { }, Deployed: true, }, - Locked: true, Default: false, } diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 335beba72..946ea4335 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -90,6 +90,9 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { accHash util.Uint160 err error ) + if a.Locked { + return errors.New("account is locked") + } if a.Contract == nil { return errors.New("account has no contract") } @@ -119,7 +122,7 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { return nil } if a.privateKey == nil { - return errors.New("account is not unlocked") + return errors.New("account key is not available (need to decrypt?)") } sign := a.privateKey.SignHashable(uint32(net), t) diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index 1e328a857..eeaa9a3f8 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -134,6 +134,10 @@ func TestContractSignTx(t *testing.T) { require.Equal(t, 1, len(tx.Scripts)) require.Equal(t, 66, len(tx.Scripts[0].InvocationScript)) + acc2.Locked = true + require.Error(t, acc2.SignTx(0, tx)) // Locked account. + + acc2.Locked = false acc2.privateKey = nil require.Error(t, acc2.SignTx(0, tx)) // No private key. From fe50879bb78e5302a4514d11f4a436d8a400ba24 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 24 Aug 2022 18:50:31 +0300 Subject: [PATCH 5/8] wallet: add (*Account).CanSign API --- pkg/wallet/account.go | 6 ++++++ pkg/wallet/account_test.go | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 946ea4335..4abc659f1 100644 --- a/pkg/wallet/account.go +++ b/pkg/wallet/account.go @@ -136,6 +136,12 @@ func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error { return nil } +// 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 { + return !a.Locked && a.privateKey != nil +} + // GetVerificationScript returns account's verification script. func (a *Account) GetVerificationScript() []byte { if a.Contract != nil { diff --git a/pkg/wallet/account_test.go b/pkg/wallet/account_test.go index eeaa9a3f8..2d0a798df 100644 --- a/pkg/wallet/account_test.go +++ b/pkg/wallet/account_test.go @@ -86,6 +86,7 @@ func TestContract_MarshalJSON(t *testing.T) { func TestContractSignTx(t *testing.T) { acc, err := NewAccount() require.NoError(t, err) + require.True(t, acc.CanSign()) accNoContr := *acc accNoContr.Contract = nil @@ -100,6 +101,7 @@ func TestContractSignTx(t *testing.T) { acc2, err := NewAccount() require.NoError(t, err) + require.True(t, acc2.CanSign()) require.Error(t, acc2.SignTx(0, tx)) @@ -135,10 +137,12 @@ func TestContractSignTx(t *testing.T) { require.Equal(t, 66, len(tx.Scripts[0].InvocationScript)) acc2.Locked = true + require.False(t, acc2.CanSign()) require.Error(t, acc2.SignTx(0, tx)) // Locked account. acc2.Locked = false acc2.privateKey = nil + require.False(t, acc2.CanSign()) require.Error(t, acc2.SignTx(0, tx)) // No private key. tx.Scripts = append(tx.Scripts, transaction.Witness{ From a95984febf1972c435b0745d42e085683b6d3b5e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 25 Aug 2022 16:40:32 +0300 Subject: [PATCH 6/8] actor: allow providing default attributes/hooks to be used Which expands Actor use cases greatly. --- pkg/rpcclient/actor/actor.go | 50 +++++++++++++++- pkg/rpcclient/actor/actor_test.go | 11 ++++ pkg/rpcclient/actor/maker.go | 95 +++++++++++++++++++------------ pkg/rpcclient/actor/maker_test.go | 39 +++++++++++++ 4 files changed, 158 insertions(+), 37 deletions(-) diff --git a/pkg/rpcclient/actor/actor.go b/pkg/rpcclient/actor/actor.go index 07b436227..aed020c7f 100644 --- a/pkg/rpcclient/actor/actor.go +++ b/pkg/rpcclient/actor/actor.go @@ -48,16 +48,35 @@ type Actor struct { invoker.Invoker client RPCActor + opts Options signers []SignerAccount txSigners []transaction.Signer version *result.Version } +// Options are used to create Actor with non-standard transaction checkers or +// additional attributes to be applied for all transactions. +type Options struct { + // Attributes are set as is into every transaction created by Actor, + // unless they're explicitly set in a method call that accepts + // attributes (like MakeTuned* or MakeUnsigned*). + Attributes []transaction.Attribute + // CheckerModifier is used by any method that creates and signs a + // transaction inside (some of them provide ways to override this + // default, some don't). + CheckerModifier TransactionCheckerModifier + // Modifier is used only by MakeUncheckedRun to modify transaction + // before it's signed (other methods that perform test invocations + // use CheckerModifier). MakeUnsigned* methods do not run it. + Modifier TransactionModifier +} + // New creates an Actor instance using the specified RPC interface and the set of // signers with corresponding accounts. Every transaction created by this Actor // will have this set of signers and all communication will be performed via this // RPC. Upon Actor instance creation a GetVersion call is made and the result of -// it is cached forever (and used for internal purposes). +// it is cached forever (and used for internal purposes). The actor will use +// default Options (which can be overridden using NewTuned). func New(ra RPCActor, signers []SignerAccount) (*Actor, error) { if len(signers) < 1 { return nil, errors.New("at least one signer (sender) is required") @@ -81,6 +100,7 @@ func New(ra RPCActor, signers []SignerAccount) (*Actor, error) { return &Actor{ Invoker: *inv, client: ra, + opts: NewDefaultOptions(), signers: signers, txSigners: invSigners, version: version, @@ -100,6 +120,34 @@ func NewSimple(ra RPCActor, acc *wallet.Account) (*Actor, error) { }}) } +// NewDefaultOptions returns Options that have no attributes and use the default +// TransactionCheckerModifier function (that checks for the invocation result to +// be in HALT state) and TransactionModifier (that does nothing). +func NewDefaultOptions() Options { + return Options{ + CheckerModifier: DefaultCheckerModifier, + Modifier: DefaultModifier, + } +} + +// NewTuned creates an Actor that will use the specified Options as defaults when +// creating new transactions. If checker/modifier callbacks are not provided +// (nil), then default ones (from NewDefaultOptions) are used. +func NewTuned(ra RPCActor, signers []SignerAccount, opts Options) (*Actor, error) { + a, err := New(ra, signers) + if err != nil { + return nil, err + } + a.opts.Attributes = opts.Attributes + if opts.CheckerModifier != nil { + a.opts.CheckerModifier = opts.CheckerModifier + } + if opts.Modifier != nil { + a.opts.Modifier = opts.Modifier + } + return a, err +} + // CalculateNetworkFee wraps RPCActor's CalculateNetworkFee, making it available // to Actor users directly. It returns network fee value for the given // transaction. diff --git a/pkg/rpcclient/actor/actor_test.go b/pkg/rpcclient/actor/actor_test.go index c87b127bd..5ec1b4c4f 100644 --- a/pkg/rpcclient/actor/actor_test.go +++ b/pkg/rpcclient/actor/actor_test.go @@ -78,6 +78,9 @@ func TestNew(t *testing.T) { _, err = New(client, []SignerAccount{}) require.Error(t, err) + _, err = NewTuned(client, []SignerAccount{}, NewDefaultOptions()) + require.Error(t, err) + // Good simple. a, err := NewSimple(client, acc) require.NoError(t, err) @@ -140,6 +143,14 @@ func TestNew(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, len(a.signers)) require.Equal(t, 2, len(a.txSigners)) + + // Good tuned + opts := Options{ + Attributes: []transaction.Attribute{{Type: transaction.HighPriority}}, + } + a, err = NewTuned(client, signers, opts) + require.NoError(t, err) + require.Equal(t, 1, len(a.opts.Attributes)) } func TestSimpleWrappers(t *testing.T) { diff --git a/pkg/rpcclient/actor/maker.go b/pkg/rpcclient/actor/maker.go index 392a84391..d185dd1fd 100644 --- a/pkg/rpcclient/actor/maker.go +++ b/pkg/rpcclient/actor/maker.go @@ -33,37 +33,53 @@ type TransactionCheckerModifier func(r *result.Invoke, t *transaction.Transactio // successfully accepted and executed. type TransactionModifier func(t *transaction.Transaction) error +// DefaultModifier is the default modifier, it does nothing. +func DefaultModifier(t *transaction.Transaction) error { + return nil +} + +// DefaultCheckerModifier is the default TransactionCheckerModifier, it checks +// for HALT state in the invocation result given to it and does nothing else. +func DefaultCheckerModifier(r *result.Invoke, t *transaction.Transaction) error { + if r.State != vmstate.Halt.String() { + return fmt.Errorf("script failed (%s state) due to an error: %s", r.State, r.FaultException) + } + return nil +} + // MakeCall creates a transaction that calls the given method of the given -// contract with the given parameters. Test call is performed and checked for -// HALT status, if more checks are needed or transaction should have some -// additional attributes use MakeTunedCall. +// contract with the given parameters. Test call is performed and filtered through +// Actor-configured TransactionCheckerModifier. The resulting transaction has +// Actor-configured attributes added as well. If you need to override attributes +// and/or TransactionCheckerModifier use MakeTunedCall. func (a *Actor) MakeCall(contract util.Uint160, method string, params ...interface{}) (*transaction.Transaction, error) { return a.MakeTunedCall(contract, method, nil, nil, params...) } -// MakeTunedCall creates a transaction with the given attributes that calls the -// given method of the given contract with the given parameters. It's filtered -// through the provided callback (see TransactionCheckerModifier documentation), -// so the process can be aborted and transaction can be modified before signing. -// If no callback is given then the result is checked for HALT state. +// MakeTunedCall creates a transaction with the given attributes (or Actor default +// ones if nil) that calls the given method of the given contract with the given +// parameters. It's filtered through the provided callback (or Actor default +// one's if nil, see TransactionCheckerModifier documentation also), so the +// process can be aborted and transaction can be modified before signing. func (a *Actor) MakeTunedCall(contract util.Uint160, method string, attrs []transaction.Attribute, txHook TransactionCheckerModifier, params ...interface{}) (*transaction.Transaction, error) { r, err := a.Call(contract, method, params...) return a.makeUncheckedWrapper(r, err, attrs, txHook) } // MakeRun creates a transaction with the given executable script. Test -// invocation of this script is performed and expected to end up in HALT -// state. If more checks are needed or transaction should have some additional -// attributes use MakeTunedRun. +// invocation of this script is performed and filtered through Actor's +// TransactionCheckerModifier. The resulting transaction has attributes that are +// configured for current Actor. If you need to override them or use a different +// TransactionCheckerModifier use MakeTunedRun. func (a *Actor) MakeRun(script []byte) (*transaction.Transaction, error) { return a.MakeTunedRun(script, nil, nil) } -// MakeTunedRun creates a transaction with the given attributes that executes -// the given script. It's filtered through the provided callback (see -// TransactionCheckerModifier documentation), so the process can be aborted and -// transaction can be modified before signing. If no callback is given then the -// result is checked for HALT state. +// MakeTunedRun creates a transaction with the given attributes (or Actor default +// ones if nil) that executes the given script. It's filtered through the +// provided callback (if not nil, otherwise Actor default one is used, see +// TransactionCheckerModifier documentation also), so the process can be aborted +// and transaction can be modified before signing. func (a *Actor) MakeTunedRun(script []byte, attrs []transaction.Attribute, txHook TransactionCheckerModifier) (*transaction.Transaction, error) { r, err := a.Run(script) return a.makeUncheckedWrapper(r, err, attrs, txHook) @@ -75,32 +91,31 @@ func (a *Actor) makeUncheckedWrapper(r *result.Invoke, err error, attrs []transa } return a.MakeUncheckedRun(r.Script, r.GasConsumed, attrs, func(tx *transaction.Transaction) error { if txHook == nil { - if r.State != vmstate.Halt.String() { - return fmt.Errorf("script failed (%s state) due to an error: %s", r.State, r.FaultException) - } - return nil + txHook = a.opts.CheckerModifier } return txHook(r, tx) }) } -// MakeUncheckedRun creates a transaction with the given attributes that executes -// the given script and is expected to use up to sysfee GAS for its execution. -// The transaction is filtered through the provided callback (see -// TransactionModifier documentation), so the process can be aborted and -// transaction can be modified before signing. This method is mostly useful when -// test invocation is already performed and the script and required system fee -// values are already known. +// MakeUncheckedRun creates a transaction with the given attributes (or Actor +// default ones if nil) that executes the given script and is expected to use +// up to sysfee GAS for its execution. The transaction is filtered through the +// provided callback (or Actor default one, see TransactionModifier documentation +// also), so the process can be aborted and transaction can be modified before +// signing. This method is mostly useful when test invocation is already +// performed and the script and required system fee values are already known. func (a *Actor) MakeUncheckedRun(script []byte, sysfee int64, attrs []transaction.Attribute, txHook TransactionModifier) (*transaction.Transaction, error) { tx, err := a.MakeUnsignedUncheckedRun(script, sysfee, attrs) if err != nil { return nil, err } - if txHook != nil { - err = txHook(tx) - if err != nil { - return nil, err - } + + if txHook == nil { + txHook = a.opts.Modifier + } + err = txHook(tx) + if err != nil { + return nil, err } err = a.Sign(tx) if err != nil { @@ -113,6 +128,8 @@ func (a *Actor) MakeUncheckedRun(script []byte, sysfee int64, attrs []transactio // that calls the given method of the given contract with the given parameters. // Test-invocation is performed and is expected to end up in HALT state, the // transaction returned has correct SystemFee and NetworkFee values. +// TransactionModifier is not applied to the result of this method, but default +// attributes are used if attrs is nil. func (a *Actor) MakeUnsignedCall(contract util.Uint160, method string, attrs []transaction.Attribute, params ...interface{}) (*transaction.Transaction, error) { r, err := a.Call(contract, method, params...) return a.makeUnsignedWrapper(r, err, attrs) @@ -121,7 +138,8 @@ func (a *Actor) MakeUnsignedCall(contract util.Uint160, method string, attrs []t // MakeUnsignedRun creates an unsigned transaction with the given attributes // that executes the given script. Test-invocation is performed and is expected // to end up in HALT state, the transaction returned has correct SystemFee and -// NetworkFee values. +// NetworkFee values. TransactionModifier is not applied to the result of this +// method, but default attributes are used if attrs is nil. func (a *Actor) MakeUnsignedRun(script []byte, attrs []transaction.Attribute) (*transaction.Transaction, error) { r, err := a.Run(script) return a.makeUnsignedWrapper(r, err, attrs) @@ -131,8 +149,9 @@ func (a *Actor) makeUnsignedWrapper(r *result.Invoke, err error, attrs []transac if err != nil { return nil, fmt.Errorf("failed to test-invoke: %w", err) } - if r.State != vmstate.Halt.String() { - return nil, fmt.Errorf("test invocation faulted (%s): %s", r.State, r.FaultException) + err = DefaultCheckerModifier(r, nil) // We know it doesn't care about transaction anyway. + if err != nil { + return nil, err } return a.MakeUnsignedUncheckedRun(r.Script, r.GasConsumed, attrs) } @@ -144,7 +163,8 @@ func (a *Actor) makeUnsignedWrapper(r *result.Invoke, err error, attrs []transac // Signers with Actor's signers, calculates proper ValidUntilBlock and NetworkFee // values. The resulting transaction can be changed in its Nonce, SystemFee, // NetworkFee and ValidUntilBlock values and then be signed and sent or -// exchanged via context.ParameterContext. +// exchanged via context.ParameterContext. TransactionModifier is not applied to +// the result of this method, but default attributes are used if attrs is nil. func (a *Actor) MakeUnsignedUncheckedRun(script []byte, sysFee int64, attrs []transaction.Attribute) (*transaction.Transaction, error) { var err error @@ -155,6 +175,9 @@ func (a *Actor) MakeUnsignedUncheckedRun(script []byte, sysFee int64, attrs []tr return nil, errors.New("negative system fee") } + if attrs == nil { + attrs = a.opts.Attributes // Might as well be nil, but it's OK. + } tx := transaction.New(script, sysFee) tx.Signers = a.txSigners tx.Attributes = attrs diff --git a/pkg/rpcclient/actor/maker_test.go b/pkg/rpcclient/actor/maker_test.go index 116baf46b..b7f66c7b4 100644 --- a/pkg/rpcclient/actor/maker_test.go +++ b/pkg/rpcclient/actor/maker_test.go @@ -90,6 +90,17 @@ func TestMakeUnsigned(t *testing.T) { client.invRes = &result.Invoke{State: "HALT", GasConsumed: 3, Script: script} _, err = a.MakeUnsignedRun(script, nil) require.NoError(t, err) + + // Tuned. + opts := Options{ + Attributes: []transaction.Attribute{{Type: transaction.HighPriority}}, + } + a, err = NewTuned(client, a.signers, opts) + require.NoError(t, err) + + tx, err = a.MakeUnsignedRun(script, nil) + require.NoError(t, err) + require.True(t, tx.HasAttribute(transaction.HighPriority)) } func TestMakeSigned(t *testing.T) { @@ -127,6 +138,20 @@ func TestMakeSigned(t *testing.T) { require.NoError(t, err) require.Equal(t, uint32(777), tx.ValidUntilBlock) + // Tuned. + opts := Options{ + Modifier: func(t *transaction.Transaction) error { + t.ValidUntilBlock = 888 + return nil + }, + } + at, err := NewTuned(client, a.signers, opts) + require.NoError(t, err) + + tx, err = at.MakeUncheckedRun(script, 0, nil, nil) + require.NoError(t, err) + require.Equal(t, uint32(888), tx.ValidUntilBlock) + // Checked // Bad, invocation fails. @@ -175,4 +200,18 @@ func TestMakeSigned(t *testing.T) { client.invRes = &result.Invoke{State: "HALT", GasConsumed: 3, Script: script} _, err = a.MakeCall(util.Uint160{}, "method", 1) require.NoError(t, err) + + // Tuned. + opts = Options{ + CheckerModifier: func(r *result.Invoke, t *transaction.Transaction) error { + t.ValidUntilBlock = 888 + return nil + }, + } + at, err = NewTuned(client, a.signers, opts) + require.NoError(t, err) + + tx, err = at.MakeRun(script) + require.NoError(t, err) + require.Equal(t, uint32(888), tx.ValidUntilBlock) } From ac5c609063bc8c0f4cce1ca12c421d933b1c5548 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 26 Aug 2022 14:47:25 +0300 Subject: [PATCH 7/8] core: add a bit more data into NVB errors It's not always obvious what they mean and the NVB value is. --- pkg/core/blockchain.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index d47c618bb..200e091e7 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -2098,17 +2098,18 @@ func (bc *Blockchain) verifyTxAttributes(d *dao.Simple, tx *transaction.Transact return fmt.Errorf("%w: NotValidBefore attribute was found, but P2PSigExtensions are disabled", ErrInvalidAttribute) } nvb := tx.Attributes[i].Value.(*transaction.NotValidBefore).Height + curHeight := bc.BlockHeight() if isPartialTx { maxNVBDelta := bc.contracts.Notary.GetMaxNotValidBeforeDelta(bc.dao) - if bc.BlockHeight()+maxNVBDelta < nvb { - return fmt.Errorf("%w: partially-filled transaction should become valid not less then %d blocks after current chain's height %d", ErrInvalidAttribute, maxNVBDelta, bc.BlockHeight()) + if curHeight+maxNVBDelta < nvb { + return fmt.Errorf("%w: NotValidBefore (%d) bigger than MaxNVBDelta (%d) allows at height %d", ErrInvalidAttribute, nvb, maxNVBDelta, curHeight) } if nvb+maxNVBDelta < tx.ValidUntilBlock { - return fmt.Errorf("%w: partially-filled transaction should be valid during less than %d blocks", ErrInvalidAttribute, maxNVBDelta) + return fmt.Errorf("%w: NotValidBefore (%d) set more than MaxNVBDelta (%d) away from VUB (%d)", ErrInvalidAttribute, nvb, maxNVBDelta, tx.ValidUntilBlock) } } else { - if height := bc.BlockHeight(); height < nvb { - return fmt.Errorf("%w: transaction is not yet valid: NotValidBefore = %d, current height = %d", ErrInvalidAttribute, nvb, height) + if curHeight < nvb { + return fmt.Errorf("%w: transaction is not yet valid: NotValidBefore = %d, current height = %d", ErrInvalidAttribute, nvb, curHeight) } } case transaction.ConflictsT: From 07f3023e84071709f27ae6d15d6f13d29261713b Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 26 Aug 2022 18:10:58 +0300 Subject: [PATCH 8/8] rpcclient: add notary.Actor for seamless notary experience --- pkg/rpcclient/notary/accounts.go | 59 +++ pkg/rpcclient/notary/accounts_test.go | 36 ++ pkg/rpcclient/notary/actor.go | 314 ++++++++++++++++ pkg/rpcclient/notary/actor_test.go | 522 ++++++++++++++++++++++++++ pkg/rpcclient/notary/contract.go | 4 +- pkg/rpcclient/rpc.go | 3 + pkg/services/rpcsrv/client_test.go | 41 +- 7 files changed, 972 insertions(+), 7 deletions(-) create mode 100644 pkg/rpcclient/notary/accounts.go create mode 100644 pkg/rpcclient/notary/accounts_test.go create mode 100644 pkg/rpcclient/notary/actor.go create mode 100644 pkg/rpcclient/notary/actor_test.go diff --git a/pkg/rpcclient/notary/accounts.go b/pkg/rpcclient/notary/accounts.go new file mode 100644 index 000000000..e2b6c6905 --- /dev/null +++ b/pkg/rpcclient/notary/accounts.go @@ -0,0 +1,59 @@ +package notary + +import ( + "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/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/wallet" +) + +// FakeSimpleAccount creates a fake account belonging to the given public key. +// It uses a simple signature contract and this account has SignTx that +// returns no error, but at the same time adds no signature (it obviously can't +// do that, so CanSign() returns false for it). Use this account for Actor when +// simple signatures are needed to be collected. +func FakeSimpleAccount(k *keys.PublicKey) *wallet.Account { + return &wallet.Account{ + Address: k.Address(), + Contract: &wallet.Contract{ + Script: k.GetVerificationScript(), + }, + } +} + +// FakeMultisigAccount creates a fake account belonging to the given "m out of +// len(pkeys)" account for the given set of keys. The account returned has SignTx +// that returns no error, but at the same time adds no signatures (it can't +// do that, so CanSign() returns false for it). Use this account for Actor when +// multisignature account needs to be added into a notary transaction, but you +// have no keys at all for it (if you have at least one (which usually is the +// case) ordinary multisig account works fine already). +func FakeMultisigAccount(m int, pkeys keys.PublicKeys) (*wallet.Account, error) { + script, err := smartcontract.CreateMultiSigRedeemScript(m, pkeys) + if err != nil { + return nil, err + } + return &wallet.Account{ + Address: address.Uint160ToString(hash.Hash160(script)), + Contract: &wallet.Contract{ + Script: script, + }, + }, nil +} + +// FakeContractAccount creates a fake account belonging to some deployed contract. +// SignTx can be called on this account with no error, but at the same time it +// adds no signature or other data into the invocation script (it obviously can't +// do that, so CanSign() returns false for it). Use this account for Actor when +// one of the signers is a contract and it doesn't need a signature or you can +// provide it externally. +func FakeContractAccount(hash util.Uint160) *wallet.Account { + return &wallet.Account{ + Address: address.Uint160ToString(hash), + Contract: &wallet.Contract{ + Deployed: true, + }, + } +} diff --git a/pkg/rpcclient/notary/accounts_test.go b/pkg/rpcclient/notary/accounts_test.go new file mode 100644 index 000000000..a87f3cace --- /dev/null +++ b/pkg/rpcclient/notary/accounts_test.go @@ -0,0 +1,36 @@ +package notary + +import ( + "testing" + + "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/stretchr/testify/require" +) + +func TestFakeAccounts(t *testing.T) { + k, err := keys.NewPrivateKey() + require.NoError(t, err) + + fac := FakeSimpleAccount(k.PublicKey()) + require.False(t, fac.CanSign()) + + sh := k.PublicKey().GetScriptHash() + tx := transaction.New([]byte{1, 2, 3}, 1) + tx.Signers = append(tx.Signers, transaction.Signer{Account: sh}) + require.NoError(t, fac.SignTx(0, tx)) + + fac = FakeContractAccount(sh) + require.False(t, fac.CanSign()) + require.NoError(t, fac.SignTx(0, tx)) + + _, err = FakeMultisigAccount(0, keys.PublicKeys{k.PublicKey()}) + require.Error(t, err) + + fac, err = FakeMultisigAccount(1, keys.PublicKeys{k.PublicKey()}) + require.NoError(t, err) + require.False(t, fac.CanSign()) + tx.Signers[0].Account = hash.Hash160(fac.Contract.Script) + require.NoError(t, fac.SignTx(0, tx)) +} diff --git a/pkg/rpcclient/notary/actor.go b/pkg/rpcclient/notary/actor.go new file mode 100644 index 000000000..b115c9b74 --- /dev/null +++ b/pkg/rpcclient/notary/actor.go @@ -0,0 +1,314 @@ +package notary + +import ( + "errors" + "fmt" + + "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/neorpc/result" + "github.com/nspcc-dev/neo-go/pkg/network/payload" + "github.com/nspcc-dev/neo-go/pkg/rpcclient/actor" + "github.com/nspcc-dev/neo-go/pkg/rpcclient/invoker" + "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" + "github.com/nspcc-dev/neo-go/pkg/wallet" +) + +// Actor encapsulates everything needed to create proper notary requests for +// assisted transactions. +type Actor struct { + // Actor is the main transaction actor, it has appropriate attributes and + // transaction modifiers to set ValidUntilBlock. Use it to create main + // transactions that have incomplete set of signatures. They can be + // signed (using available wallets), but can not be sent directly to the + // network. Instead of sending them to the network use Actor methods to + // wrap them into notary requests. + actor.Actor + // FbActor is the fallback transaction actor, it has two required signers + // and a set of attributes expected from a fallback transaction. It can + // be used to create _unsigned_ transactions with whatever actions + // required (but no additional attributes can be added). Signing them + // while technically possible (with notary contract signature missing), + // will lead to incorrect transaction because NotValidBefore and + // Conflicts attributes as well as ValidUntilBlock field can be + // correctly set only when some main transaction is available. + FbActor actor.Actor + + fbScript []byte + reader *ContractReader + sender *wallet.Account + rpc RPCActor +} + +// ActorOptions are used to influence main and fallback actors as well as the +// default Notarize behavior. +type ActorOptions struct { + // FbAttributes are additional attributes to be added into fallback + // transaction by an appropriate actor. Irrespective of this setting + // (which defaults to nil) NotaryAssisted, NotValidBefore and Conflicts + // attributes are always added. + FbAttributes []transaction.Attribute + // FbScript is the script to use in the Notarize convenience method, it + // defaults to a simple RET instruction (doing nothing). + FbScript []byte + // FbSigner is the second signer to be used for the fallback transaction. + // By default it's derived from the account and has None scope, it has + // to be a simple signature or deployed contract account, but this setting + // allows you to give it some other scope to be used in complex fallback + // scripts. + FbSigner actor.SignerAccount + // MainAttribtues are additional attributes to be added into main + // transaction by an appropriate actor. Irrespective of this setting + // (which defaults to nil) NotaryAssisted attribute is always added. + MainAttributes []transaction.Attribute + // MainCheckerModifier will be used by the main Actor when creating + // transactions. It defaults to using [actor.DefaultCheckerModifier] + // for result check and adds MaxNotValidBeforeDelta to the + // ValidUntilBlock transaction's field. Only override it if you know + // what you're doing. + MainCheckerModifier actor.TransactionCheckerModifier + // MainModifier will be used by the main Actor when creating + // transactions. By default it adds MaxNotValidBeforeDelta to the + // ValidUntilBlock transaction's field. Only override it if you know + // what you're doing. + MainModifier actor.TransactionModifier +} + +// RPCActor is a set of methods required from RPC client to create Actor. +type RPCActor interface { + actor.RPCActor + + SubmitP2PNotaryRequest(req *payload.P2PNotaryRequest) (util.Uint256, error) +} + +// NewDefaultActorOptions returns the default Actor options. Internal functions +// of it need some data from the contract, so it should be added. +func NewDefaultActorOptions(reader *ContractReader, acc *wallet.Account) ActorOptions { + opts := ActorOptions{ + FbScript: []byte{byte(opcode.RET)}, + FbSigner: actor.SignerAccount{ + Signer: transaction.Signer{ + Account: acc.Contract.ScriptHash(), + Scopes: transaction.None, + }, + Account: acc, + }, + MainModifier: func(t *transaction.Transaction) error { + nvbDelta, err := reader.GetMaxNotValidBeforeDelta() + if err != nil { + return fmt.Errorf("can't get MaxNVBDelta: %w", err) + } + t.ValidUntilBlock += nvbDelta + return nil + }, + } + opts.MainCheckerModifier = func(r *result.Invoke, t *transaction.Transaction) error { + err := actor.DefaultCheckerModifier(r, t) + if err != nil { + return err + } + return opts.MainModifier(t) + } + return opts +} + +// NewActor creates a new notary.Actor using the given RPC client, the set of +// signers for main transactions and the account that will sign notary requests +// (one plain signature or contract-based). The set of signers will be extended +// by the notary contract signer with the None scope (as required by the notary +// protocol) and all transactions created with the resulting Actor will get a +// NotaryAssisted attribute with appropriate number of keys specified +// (depending on signers). A fallback Actor will be created as well with the +// notary contract and simpleAcc signers and a full set of required fallback +// transaction attributes (NotaryAssisted, NotValidBefore and Conflicts). +func NewActor(c RPCActor, signers []actor.SignerAccount, simpleAcc *wallet.Account) (*Actor, error) { + return newTunedActor(c, signers, simpleAcc, nil) +} + +// NewTunedActor is the same as NewActor, but allows to override the default +// options (see ActorOptions for details). Use with care. +func NewTunedActor(c RPCActor, signers []actor.SignerAccount, opts ActorOptions) (*Actor, error) { + return newTunedActor(c, signers, opts.FbSigner.Account, &opts) +} + +func newTunedActor(c RPCActor, signers []actor.SignerAccount, simpleAcc *wallet.Account, opts *ActorOptions) (*Actor, error) { + if len(signers) < 1 { + return nil, errors.New("at least one signer (sender) is required") + } + var nKeys int + for _, sa := range signers { + if sa.Account.Contract == nil { + return nil, fmt.Errorf("empty contract for account %s", sa.Account.Address) + } + if sa.Account.Contract.Deployed { + continue + } + if vm.IsSignatureContract(sa.Account.Contract.Script) { + nKeys++ + continue + } + _, pubs, ok := vm.ParseMultiSigContract(sa.Account.Contract.Script) + if !ok { + return nil, fmt.Errorf("signer %s is not a contract- or signature-based", sa.Account.Address) + } + nKeys += len(pubs) + } + if nKeys > 255 { + return nil, fmt.Errorf("notary subsystem can't handle more than 255 signatures") + } + if simpleAcc.Contract == nil { + return nil, errors.New("bad simple account: no contract") + } + if !simpleAcc.CanSign() { + return nil, errors.New("bad simple account: can't sign") + } + if !vm.IsSignatureContract(simpleAcc.Contract.Script) && !simpleAcc.Contract.Deployed { + return nil, errors.New("bad simple account: neither plain signature, nor contract") + } + // Not reusing mainActor/fbActor for ContractReader to make requests a bit lighter. + reader := NewReader(invoker.New(c, nil)) + if opts == nil { + defOpts := NewDefaultActorOptions(reader, simpleAcc) + opts = &defOpts + } + var notarySA = actor.SignerAccount{ + Signer: transaction.Signer{ + Account: Hash, + Scopes: transaction.None, + }, + Account: FakeContractAccount(Hash), + } + + var mainSigners = make([]actor.SignerAccount, len(signers), len(signers)+1) + copy(mainSigners, signers) + mainSigners = append(mainSigners, notarySA) + + mainOpts := actor.Options{ + Attributes: []transaction.Attribute{{ + Type: transaction.NotaryAssistedT, + Value: &transaction.NotaryAssisted{NKeys: uint8(nKeys)}, + }}, + CheckerModifier: opts.MainCheckerModifier, + Modifier: opts.MainModifier, + } + mainOpts.Attributes = append(mainOpts.Attributes, opts.MainAttributes...) + + mainActor, err := actor.NewTuned(c, mainSigners, mainOpts) + if err != nil { + return nil, err + } + + fbSigners := []actor.SignerAccount{notarySA, opts.FbSigner} + fbOpts := actor.Options{ + Attributes: []transaction.Attribute{{ + Type: transaction.NotaryAssistedT, + Value: &transaction.NotaryAssisted{NKeys: 0}, + }, { + // A stub, it has correct size, but the contents is to be filled per-request. + Type: transaction.NotValidBeforeT, + Value: &transaction.NotValidBefore{}, + }, { + // A stub, it has correct size, but the contents is to be filled per-request. + Type: transaction.ConflictsT, + Value: &transaction.Conflicts{}, + }}, + } + fbOpts.Attributes = append(fbOpts.Attributes, opts.FbAttributes...) + fbActor, err := actor.NewTuned(c, fbSigners, fbOpts) + if err != nil { + return nil, err + } + return &Actor{*mainActor, *fbActor, opts.FbScript, reader, simpleAcc, c}, nil +} + +// Notarize is a simple wrapper for transaction-creating functions that allows to +// send any partially-signed transaction in a notary request with a fallback +// transaction created based on Actor settings and SendRequest adjustment rules. +// The values returned are main and fallback transaction hashes, ValidUntilBlock +// and error if any. +func (a *Actor) Notarize(mainTx *transaction.Transaction, err error) (util.Uint256, util.Uint256, uint32, error) { + var ( + // Just to simplify return values on error. + fbHash util.Uint256 + mainHash util.Uint256 + vub uint32 + ) + if err != nil { + return mainHash, fbHash, vub, err + } + fbTx, err := a.FbActor.MakeUnsignedRun(a.fbScript, nil) + if err != nil { + return mainHash, fbHash, vub, err + } + return a.SendRequest(mainTx, fbTx) +} + +// SendRequest creates and sends a notary request using the given main and +// fallback transactions. It accepts signed main transaction and unsigned fallback +// transaction that will be adjusted in its NotValidBefore and Conflicts +// attributes as well as ValidUntilBlock value. Conflicts is set to the main +// transaction hash, while NotValidBefore is set to the middle of current mainTx +// lifetime (between current block and ValidUntilBlock). The values returned are +// main and fallback transaction hashes, ValidUntilBlock and error if any. +func (a *Actor) SendRequest(mainTx *transaction.Transaction, fbTx *transaction.Transaction) (util.Uint256, util.Uint256, uint32, error) { + var ( + fbHash util.Uint256 + mainHash = mainTx.Hash() + vub = mainTx.ValidUntilBlock + ) + if len(fbTx.Attributes) < 3 { + return mainHash, fbHash, vub, errors.New("invalid fallback: missing required attributes") + } + if fbTx.Attributes[1].Type != transaction.NotValidBeforeT { + return mainHash, fbHash, vub, errors.New("invalid fallback: NotValidBefore is missing where expected") + } + if fbTx.Attributes[2].Type != transaction.ConflictsT { + return mainHash, fbHash, vub, errors.New("invalid fallback: Conflicts is missing where expected") + } + height, err := a.GetBlockCount() + if err != nil { + return mainHash, fbHash, vub, err + } + // New values must be created to avoid overwriting originals via a pointer. + fbTx.Attributes[1].Value = &transaction.NotValidBefore{Height: (height + vub) / 2} + fbTx.Attributes[2].Value = &transaction.Conflicts{Hash: mainHash} + fbTx.ValidUntilBlock = vub + err = a.FbActor.Sign(fbTx) + if err != nil { + return mainHash, fbHash, vub, err + } + fbTx.Scripts[0].InvocationScript = append([]byte{byte(opcode.PUSHDATA1), 64}, make([]byte, 64)...) // Must be present. + return a.SendRequestExactly(mainTx, fbTx) +} + +// SendRequestExactly accepts signed and completely prepared main and fallback +// transactions, creates a P2P notary request containing them, signs and sends +// it to the network. Caller takes full responsibility for transaction +// correctness in this case, use this method only if you know exactly that you +// need to override some of the other method's behavior and you can do it. The +// values returned are main and fallback transaction hashes, ValidUntilBlock +// and error if any. +func (a *Actor) SendRequestExactly(mainTx *transaction.Transaction, fbTx *transaction.Transaction) (util.Uint256, util.Uint256, uint32, error) { + var ( + fbHash = fbTx.Hash() + mainHash = mainTx.Hash() + vub = mainTx.ValidUntilBlock + ) + req := &payload.P2PNotaryRequest{ + MainTransaction: mainTx, + FallbackTransaction: fbTx, + } + req.Witness = transaction.Witness{ + InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, a.sender.PrivateKey().SignHashable(uint32(a.GetNetwork()), req)...), + VerificationScript: a.sender.GetVerificationScript(), + } + actualHash, err := a.rpc.SubmitP2PNotaryRequest(req) + if err != nil { + return mainHash, fbHash, vub, fmt.Errorf("failed to submit notary request: %w", err) + } + if !actualHash.Equals(fbHash) { + return mainHash, fbHash, vub, fmt.Errorf("sent and actual fallback tx hashes mismatch: %v vs %v", fbHash.StringLE(), actualHash.StringLE()) + } + return mainHash, fbHash, vub, nil +} diff --git a/pkg/rpcclient/notary/actor_test.go b/pkg/rpcclient/notary/actor_test.go new file mode 100644 index 000000000..c3482c75e --- /dev/null +++ b/pkg/rpcclient/notary/actor_test.go @@ -0,0 +1,522 @@ +package notary + +import ( + "errors" + "testing" + + "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/crypto/keys" + "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/network/payload" + "github.com/nspcc-dev/neo-go/pkg/rpcclient/actor" + "github.com/nspcc-dev/neo-go/pkg/rpcclient/invoker" + "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/nspcc-dev/neo-go/pkg/wallet" + "github.com/stretchr/testify/require" +) + +type RPCClient struct { + err error + invRes *result.Invoke + netFee int64 + bCount uint32 + version *result.Version + hash util.Uint256 + nhash util.Uint256 + mirror bool +} + +func (r *RPCClient) InvokeContractVerify(contract util.Uint160, params []smartcontract.Parameter, signers []transaction.Signer, witnesses ...transaction.Witness) (*result.Invoke, error) { + return r.invRes, r.err +} +func (r *RPCClient) InvokeFunction(contract util.Uint160, operation string, params []smartcontract.Parameter, signers []transaction.Signer) (*result.Invoke, error) { + return r.invRes, r.err +} +func (r *RPCClient) InvokeScript(script []byte, signers []transaction.Signer) (*result.Invoke, error) { + return r.invRes, r.err +} +func (r *RPCClient) CalculateNetworkFee(tx *transaction.Transaction) (int64, error) { + return r.netFee, r.err +} +func (r *RPCClient) GetBlockCount() (uint32, error) { + return r.bCount, r.err +} +func (r *RPCClient) GetVersion() (*result.Version, error) { + verCopy := *r.version + return &verCopy, r.err +} +func (r *RPCClient) SendRawTransaction(tx *transaction.Transaction) (util.Uint256, error) { + return r.hash, r.err +} +func (r *RPCClient) SubmitP2PNotaryRequest(req *payload.P2PNotaryRequest) (util.Uint256, error) { + if r.mirror { + return req.FallbackTransaction.Hash(), nil + } + return r.nhash, r.err +} +func (r *RPCClient) TerminateSession(sessionID uuid.UUID) (bool, error) { + return false, nil // Just a stub, unused by actor. +} +func (r *RPCClient) TraverseIterator(sessionID, iteratorID uuid.UUID, maxItemsCount int) ([]stackitem.Item, error) { + return nil, nil // Just a stub, unused by actor. +} + +func TestNewActor(t *testing.T) { + rc := &RPCClient{ + version: &result.Version{ + Protocol: result.Protocol{ + Network: netmode.UnitTestNet, + MillisecondsPerBlock: 1000, + ValidatorsCount: 7, + }, + }, + } + + _, err := NewActor(rc, nil, nil) + require.Error(t, err) + + var ( + keyz [4]*keys.PrivateKey + accs [4]*wallet.Account + faccs [4]*wallet.Account + pkeys [4]*keys.PublicKey + ) + for i := range accs { + keyz[i], err = keys.NewPrivateKey() + require.NoError(t, err) + accs[i] = wallet.NewAccountFromPrivateKey(keyz[i]) + pkeys[i] = keyz[i].PublicKey() + faccs[i] = FakeSimpleAccount(pkeys[i]) + } + var multiAccs [4]*wallet.Account + for i := range accs { + multiAccs[i] = &wallet.Account{} + *multiAccs[i] = *accs[i] + require.NoError(t, multiAccs[i].ConvertMultisig(smartcontract.GetDefaultHonestNodeCount(len(pkeys)), pkeys[:])) + } + + // nil Contract + badMultiAcc0 := &wallet.Account{} + *badMultiAcc0 = *multiAccs[0] + badMultiAcc0.Contract = nil + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAccs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: badMultiAcc0, + }}, accs[0]) + require.Error(t, err) + + // Non-standard script. + badMultiAcc0.Contract = &wallet.Contract{} + *badMultiAcc0.Contract = *multiAccs[0].Contract + badMultiAcc0.Contract.Script = append(badMultiAcc0.Contract.Script, byte(opcode.NOP)) + badMultiAcc0.Address = address.Uint160ToString(badMultiAcc0.Contract.ScriptHash()) + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: badMultiAcc0.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: badMultiAcc0, + }}, accs[0]) + require.Error(t, err) + + // Too many keys + var ( + manyKeys [256]*keys.PrivateKey + manyPkeys [256]*keys.PublicKey + ) + for i := range manyKeys { + manyKeys[i], err = keys.NewPrivateKey() + require.NoError(t, err) + manyPkeys[i] = manyKeys[i].PublicKey() + } + bigMultiAcc := &wallet.Account{} + *bigMultiAcc = *wallet.NewAccountFromPrivateKey(manyKeys[0]) + require.NoError(t, bigMultiAcc.ConvertMultisig(129, manyPkeys[:])) + + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: bigMultiAcc.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: bigMultiAcc, + }}, wallet.NewAccountFromPrivateKey(manyKeys[0])) + require.Error(t, err) + + // No contract in the simple account. + badSimple0 := &wallet.Account{} + *badSimple0 = *accs[0] + badSimple0.Contract = nil + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAccs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAccs[0], + }}, badSimple0) + require.Error(t, err) + + // Simple account that can't sign. + badSimple0 = FakeSimpleAccount(pkeys[0]) + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAccs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAccs[0], + }}, badSimple0) + require.Error(t, err) + + // Multisig account instead of simple one. + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAccs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAccs[0], + }}, multiAccs[0]) + require.Error(t, err) + + // Main actor freaking out on hash mismatch. + _, err = NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: accs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAccs[0], + }}, accs[0]) + require.Error(t, err) + + // FB actor freaking out on hash mismatch. + opts := NewDefaultActorOptions(NewReader(invoker.New(rc, nil)), accs[0]) + opts.FbSigner.Signer.Account = multiAccs[0].Contract.ScriptHash() + _, err = NewTunedActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAccs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAccs[0], + }}, opts) + require.Error(t, err) + + // Good, one multisig. + multi0, err := NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAccs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAccs[0], + }}, accs[0]) + require.NoError(t, err) + + script := []byte{byte(opcode.RET)} + rc.invRes = &result.Invoke{ + State: "HALT", + GasConsumed: 3, + Script: script, + Stack: []stackitem.Item{stackitem.Make(42)}, + } + tx, err := multi0.MakeRun(script) + require.NoError(t, err) + require.Equal(t, 1, len(tx.Attributes)) + require.Equal(t, transaction.NotaryAssistedT, tx.Attributes[0].Type) + require.Equal(t, &transaction.NotaryAssisted{NKeys: 4}, tx.Attributes[0].Value) + + // Good, 4 single sigs with one that can sign and one contract. + single4, err := NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: accs[0].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: accs[0], + }, { + Signer: transaction.Signer{ + Account: faccs[1].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: faccs[1], + }, { + Signer: transaction.Signer{ + Account: faccs[2].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: faccs[2], + }, { + Signer: transaction.Signer{ + Account: accs[3].Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: faccs[3], + }, { + Signer: transaction.Signer{ + Account: util.Uint160{1, 2, 3}, + Scopes: transaction.CalledByEntry, + }, + Account: FakeContractAccount(util.Uint160{1, 2, 3}), + }}, accs[0]) + require.NoError(t, err) + + tx, err = single4.MakeRun(script) + require.NoError(t, err) + require.Equal(t, 1, len(tx.Attributes)) + require.Equal(t, transaction.NotaryAssistedT, tx.Attributes[0].Type) + require.Equal(t, &transaction.NotaryAssisted{NKeys: 4}, tx.Attributes[0].Value) // One account can sign, three need to collect additional sigs. +} + +func TestSendRequestExactly(t *testing.T) { + rc := &RPCClient{ + version: &result.Version{ + Protocol: result.Protocol{ + Network: netmode.UnitTestNet, + MillisecondsPerBlock: 1000, + ValidatorsCount: 7, + }, + }, + } + + key0, err := keys.NewPrivateKey() + require.NoError(t, err) + key1, err := keys.NewPrivateKey() + require.NoError(t, err) + + acc0 := wallet.NewAccountFromPrivateKey(key0) + facc1 := FakeSimpleAccount(key1.PublicKey()) + + act, err := NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: acc0.Contract.ScriptHash(), + Scopes: transaction.None, + }, + Account: acc0, + }, { + Signer: transaction.Signer{ + Account: facc1.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: facc1, + }}, acc0) + require.NoError(t, err) + + script := []byte{byte(opcode.RET)} + mainTx := transaction.New(script, 1) + fbTx := transaction.New(script, 1) + + // Hashes mismatch + _, _, _, err = act.SendRequestExactly(mainTx, fbTx) + require.Error(t, err) + + // Error returned + rc.err = errors.New("") + _, _, _, err = act.SendRequestExactly(mainTx, fbTx) + require.Error(t, err) + + // OK returned + rc.err = nil + rc.nhash = fbTx.Hash() + mHash, fbHash, vub, err := act.SendRequestExactly(mainTx, fbTx) + require.NoError(t, err) + require.Equal(t, mainTx.Hash(), mHash) + require.Equal(t, fbTx.Hash(), fbHash) + require.Equal(t, mainTx.ValidUntilBlock, vub) +} + +func TestSendRequest(t *testing.T) { + rc := &RPCClient{ + version: &result.Version{ + Protocol: result.Protocol{ + Network: netmode.UnitTestNet, + MillisecondsPerBlock: 1000, + ValidatorsCount: 7, + }, + }, + bCount: 42, + } + + key0, err := keys.NewPrivateKey() + require.NoError(t, err) + key1, err := keys.NewPrivateKey() + require.NoError(t, err) + + acc0 := wallet.NewAccountFromPrivateKey(key0) + facc0 := FakeSimpleAccount(key0.PublicKey()) + facc1 := FakeSimpleAccount(key1.PublicKey()) + + act, err := NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: acc0.Contract.ScriptHash(), + Scopes: transaction.None, + }, + Account: acc0, + }, { + Signer: transaction.Signer{ + Account: facc1.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: facc1, + }}, acc0) + require.NoError(t, err) + + script := []byte{byte(opcode.RET)} + rc.invRes = &result.Invoke{ + State: "HALT", + GasConsumed: 3, + Script: script, + Stack: []stackitem.Item{stackitem.Make(42)}, + } + + mainTx, err := act.MakeRun(script) + require.NoError(t, err) + + // No attributes. + fbTx, err := act.FbActor.MakeUnsignedRun(script, nil) + require.NoError(t, err) + fbTx.Attributes = nil + _, _, _, err = act.SendRequest(mainTx, fbTx) + require.Error(t, err) + + // Bad NVB. + fbTx, err = act.FbActor.MakeUnsignedRun(script, nil) + require.NoError(t, err) + fbTx.Attributes[1].Type = transaction.HighPriority + fbTx.Attributes[1].Value = nil + _, _, _, err = act.SendRequest(mainTx, fbTx) + require.Error(t, err) + + // Bad Conflicts. + fbTx, err = act.FbActor.MakeUnsignedRun(script, nil) + require.NoError(t, err) + fbTx.Attributes[2].Type = transaction.HighPriority + fbTx.Attributes[2].Value = nil + _, _, _, err = act.SendRequest(mainTx, fbTx) + require.Error(t, err) + + // GetBlockCount error. + fbTx, err = act.FbActor.MakeUnsignedRun(script, nil) + require.NoError(t, err) + rc.err = errors.New("") + _, _, _, err = act.SendRequest(mainTx, fbTx) + require.Error(t, err) + + // Can't sign suddenly. + rc.err = nil + acc0Backup := &wallet.Account{} + *acc0Backup = *acc0 + *acc0 = *facc0 + fbTx, err = act.FbActor.MakeUnsignedRun(script, nil) + require.NoError(t, err) + _, _, _, err = act.SendRequest(mainTx, fbTx) + require.Error(t, err) + + // Good. + *acc0 = *acc0Backup + fbTx, err = act.FbActor.MakeUnsignedRun(script, nil) + require.NoError(t, err) + _, _, _, err = act.SendRequest(mainTx, fbTx) + require.Error(t, err) +} + +func TestNotarize(t *testing.T) { + rc := &RPCClient{ + version: &result.Version{ + Protocol: result.Protocol{ + Network: netmode.UnitTestNet, + MillisecondsPerBlock: 1000, + ValidatorsCount: 7, + }, + }, + bCount: 42, + } + + key0, err := keys.NewPrivateKey() + require.NoError(t, err) + key1, err := keys.NewPrivateKey() + require.NoError(t, err) + + acc0 := wallet.NewAccountFromPrivateKey(key0) + facc1 := FakeSimpleAccount(key1.PublicKey()) + + act, err := NewActor(rc, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: acc0.Contract.ScriptHash(), + Scopes: transaction.None, + }, + Account: acc0, + }, { + Signer: transaction.Signer{ + Account: facc1.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: facc1, + }}, acc0) + require.NoError(t, err) + + script := []byte{byte(opcode.RET)} + + // Immediate error from MakeRun. + rc.invRes = &result.Invoke{ + State: "FAULT", + GasConsumed: 3, + Script: script, + Stack: []stackitem.Item{stackitem.Make(42)}, + } + _, _, _, err = act.Notarize(act.MakeRun(script)) + require.Error(t, err) + + // Explicitly good transaction. but failure to create a fallback. + rc.invRes.State = "HALT" + tx, err := act.MakeRun(script) + require.NoError(t, err) + + rc.invRes.State = "FAULT" + _, _, _, err = act.Notarize(tx, nil) + require.Error(t, err) + + // FB hash mismatch from SendRequestExactly. + rc.invRes.State = "HALT" + _, _, _, err = act.Notarize(act.MakeRun(script)) + require.Error(t, err) + + // Good. + rc.mirror = true + mHash, fbHash, vub, err := act.Notarize(act.MakeRun(script)) + require.NoError(t, err) + require.NotEqual(t, util.Uint256{}, mHash) + require.NotEqual(t, util.Uint256{}, fbHash) + require.Equal(t, uint32(92), vub) +} + +func TestDefaultActorOptions(t *testing.T) { + rc := &RPCClient{ + version: &result.Version{ + Protocol: result.Protocol{ + Network: netmode.UnitTestNet, + MillisecondsPerBlock: 1000, + ValidatorsCount: 7, + }, + }, + } + acc, err := wallet.NewAccount() + require.NoError(t, err) + opts := NewDefaultActorOptions(NewReader(invoker.New(rc, nil)), acc) + rc.invRes = &result.Invoke{ + State: "HALT", + GasConsumed: 3, + Script: opts.FbScript, + Stack: []stackitem.Item{stackitem.Make(42)}, + } + tx := transaction.New(opts.FbScript, 1) + require.Error(t, opts.MainCheckerModifier(&result.Invoke{State: "FAULT"}, tx)) + rc.invRes.State = "FAULT" + require.Error(t, opts.MainCheckerModifier(&result.Invoke{State: "HALT"}, tx)) + rc.invRes.State = "HALT" + require.NoError(t, opts.MainCheckerModifier(&result.Invoke{State: "HALT"}, tx)) + require.Equal(t, uint32(42), tx.ValidUntilBlock) +} diff --git a/pkg/rpcclient/notary/contract.go b/pkg/rpcclient/notary/contract.go index aa216cbd4..eee21bd0a 100644 --- a/pkg/rpcclient/notary/contract.go +++ b/pkg/rpcclient/notary/contract.go @@ -2,8 +2,8 @@ Package notary provides an RPC-based wrapper for the Notary subsystem. It provides both regular ContractReader/Contract interfaces for the notary -contract and notary-specific functions and interfaces to simplify creation of -notary requests. +contract and notary-specific Actor as well as some helper functions to simplify +creation of notary requests. */ package notary diff --git a/pkg/rpcclient/rpc.go b/pkg/rpcclient/rpc.go index 66dc33840..4de2a57c0 100644 --- a/pkg/rpcclient/rpc.go +++ b/pkg/rpcclient/rpc.go @@ -866,6 +866,9 @@ func getSigners(sender *wallet.Account, cosigners []SignerAccount) ([]transactio // can be multisignature), or it only should have a partial multisignature. // // Note: client should be initialized before SignAndPushP2PNotaryRequest call. +// +// Deprecated: please use Actor from the notary subpackage. This method will be +// deleted in future versions. func (c *Client) SignAndPushP2PNotaryRequest(mainTx *transaction.Transaction, fallbackScript []byte, fallbackSysFee int64, fallbackNetFee int64, fallbackValidFor uint32, acc *wallet.Account) (*payload.P2PNotaryRequest, error) { var err error notaryHash, err := c.GetNativeContractHash(nativenames.Notary) diff --git a/pkg/services/rpcsrv/client_test.go b/pkg/services/rpcsrv/client_test.go index 5315c8f16..43f6613ff 100644 --- a/pkg/services/rpcsrv/client_test.go +++ b/pkg/services/rpcsrv/client_test.go @@ -985,6 +985,37 @@ func TestSignAndPushInvocationTx(t *testing.T) { }) } +func TestNotaryActor(t *testing.T) { + chain, rpcSrv, httpSrv := initServerWithInMemoryChainAndServices(t, false, true, false) + defer chain.Close() + defer rpcSrv.Shutdown() + + c, err := rpcclient.New(context.Background(), httpSrv.URL, rpcclient.Options{}) + require.NoError(t, err) + + sender := testchain.PrivateKeyByID(0) // owner of the deposit in testchain + acc := wallet.NewAccountFromPrivateKey(sender) + + comm, err := c.GetCommittee() + require.NoError(t, err) + + multiAcc := &wallet.Account{} + *multiAcc = *acc + require.NoError(t, multiAcc.ConvertMultisig(smartcontract.GetMajorityHonestNodeCount(len(comm)), comm)) + + nact, err := notary.NewActor(c, []actor.SignerAccount{{ + Signer: transaction.Signer{ + Account: multiAcc.Contract.ScriptHash(), + Scopes: transaction.CalledByEntry, + }, + Account: multiAcc, + }}, acc) + require.NoError(t, err) + neoW := neo.New(nact) + _, _, _, err = nact.Notarize(neoW.SetRegisterPriceTransaction(1_0000_0000)) + require.NoError(t, err) +} + func TestSignAndPushP2PNotaryRequest(t *testing.T) { chain, rpcSrv, httpSrv := initServerWithInMemoryChainAndServices(t, false, true, false) defer chain.Close() @@ -996,23 +1027,23 @@ func TestSignAndPushP2PNotaryRequest(t *testing.T) { require.NoError(t, err) t.Run("client wasn't initialized", func(t *testing.T) { - _, err := c.SignAndPushP2PNotaryRequest(transaction.New([]byte{byte(opcode.RET)}, 123), []byte{byte(opcode.RET)}, -1, 0, 100, acc) + _, err := c.SignAndPushP2PNotaryRequest(transaction.New([]byte{byte(opcode.RET)}, 123), []byte{byte(opcode.RET)}, -1, 0, 100, acc) //nolint:staticcheck // SA1019: c.SignAndPushP2PNotaryRequest is deprecated require.NotNil(t, err) }) 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"}) + _, 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) + _, err := c.SignAndPushP2PNotaryRequest(nil, []byte{byte(opcode.ASSERT)}, -1, 0, 0, acc) //nolint:staticcheck // SA1019: c.SignAndPushP2PNotaryRequest is deprecated require.NotNil(t, err) }) t.Run("too large fallbackValidFor", func(t *testing.T) { - _, err := c.SignAndPushP2PNotaryRequest(nil, []byte{byte(opcode.RET)}, -1, 0, 141, acc) + _, err := c.SignAndPushP2PNotaryRequest(nil, []byte{byte(opcode.RET)}, -1, 0, 141, acc) //nolint:staticcheck // SA1019: c.SignAndPushP2PNotaryRequest is deprecated require.NotNil(t, err) }) @@ -1031,7 +1062,7 @@ func TestSignAndPushP2PNotaryRequest(t *testing.T) { } mainTx := expected _ = expected.Hash() - req, err := c.SignAndPushP2PNotaryRequest(&mainTx, []byte{byte(opcode.RET)}, -1, 0, 6, acc) + req, err := c.SignAndPushP2PNotaryRequest(&mainTx, []byte{byte(opcode.RET)}, -1, 0, 6, acc) //nolint:staticcheck // SA1019: c.SignAndPushP2PNotaryRequest is deprecated require.NoError(t, err) // check that request was correctly completed