TrueCloudLab/neoneo-go
7
0
Fork 2
Code Issues Pull requests Projects Releases Packages Wiki Activity

rpc: carefully review places where Client.cache.initDone is used

Browse source 
1. Keep initDone check only for the places where cache is directly accessed.
   We don't need to check it in other places, otherwise we have a mess of
   duplicating checks.
2. Fix bug in code related to block deserialisation. There's no magic, so
   checking that initialisation is done is not enough for proper block
   deserialisation. We need to manually fill StateRootEnabled field.
3. Since transaction doesn't need network magic to compute its hash, we don't
   need to perform Client initialisation before transaction-related requests.
4. Check that cache is initialised before accessing network magic.
5. Refactor the way Policy contract hash is fetched for Client requests.
   We don't really need Client initialisation for that, it's OK to fetch Policy
   hash on-the-fly.
This commit is contained in:
AnnaShaleva 2022-02-21 13:41:14 +03:00 committed by Anna Shaleva
parent 7c1862a9ac
commit 0092330fe1
12 changed files with 81 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
cli/smartcontract/smart_contract.go
View file

@ -681,7 +681,11 @@ func invokeWithArgs(ctx *cli.Context, acc *wallet.Account, wall *wallet.Wallet,
if err != nil {
return sender, cli.NewExitError(fmt.Errorf("failed to create tx: %w", err), 1)
}
if err := paramcontext.InitAndSave(c.GetNetwork(), tx, acc, out); err != nil {
m, err := c.GetNetwork()
if err != nil {
return sender, cli.NewExitError(fmt.Errorf("failed to save tx: %w", err), 1)
}
if err := paramcontext.InitAndSave(m, tx, acc, out); err != nil {
return sender, cli.NewExitError(err, 1)
}
fmt.Fprintln(ctx.App.Writer, tx.Hash().StringLE())

6
cli/wallet/nep11.go
View file

@ -274,7 +274,11 @@ func signAndSendNEP11Transfer(ctx *cli.Context, c *client.Client, acc *wallet.Ac
tx.SystemFee += int64(sysgas)
if outFile := ctx.String("out"); outFile != "" {
if err := paramcontext.InitAndSave(c.GetNetwork(), tx, acc, outFile); err != nil {
m, err := c.GetNetwork()
if err != nil {
return cli.NewExitError(fmt.Errorf("failed to save tx: %w", err), 1)
}
if err := paramcontext.InitAndSave(m, tx, acc, outFile); err != nil {
return cli.NewExitError(err, 1)
}
} else {

6
cli/wallet/nep17.go
View file

@ -647,7 +647,11 @@ func signAndSendNEP17Transfer(ctx *cli.Context, c *client.Client, acc *wallet.Ac
tx.SystemFee += int64(sysgas)
if outFile := ctx.String("out"); outFile != "" {
if err := paramcontext.InitAndSave(c.GetNetwork(), tx, acc, outFile); err != nil {
m, err := c.GetNetwork()
if err != nil {
return cli.NewExitError(fmt.Errorf("failed to save tx: %w", err), 1)
}
if err := paramcontext.InitAndSave(m, tx, acc, outFile); err != nil {
return cli.NewExitError(err, 1)
}
} else {

4
pkg/rpc/client/client.go
View file

@ -116,8 +116,8 @@ func New(ctx context.Context, endpoint string, opts Options) (*Client, error) {
// Init sets magic of the network client connected to, stateRootInHeader option
// and native NEO, GAS and Policy contracts scripthashes. This method should be
// called before any transaction-, header- or block-related requests in order to
// deserialize responses properly.
// called before any header- or block-related requests in order to deserialize
// responses properly.
func (c *Client) Init() error {
version, err := c.GetVersion()
if err != nil {

6
pkg/rpc/client/nep11.go
View file

@ -46,9 +46,6 @@ func (c *Client) NEP11TokenInfo(tokenHash util.Uint160) (*wallet.Token, error) {
// given account and sends it to the network returning just a hash of it.
func (c *Client) TransferNEP11(acc *wallet.Account, to util.Uint160,
tokenHash util.Uint160, tokenID string, data interface{}, gas int64, cosigners []SignerAccount) (util.Uint256, error) {
if !c.cache.initDone {
return util.Uint256{}, errNetworkNotInitialized
}
tx, err := c.CreateNEP11TransferTx(acc, tokenHash, gas, cosigners, to, tokenID, data)
if err != nil {
return util.Uint256{}, err
@ -144,9 +141,6 @@ func (c *Client) NEP11NDOwnerOf(tokenHash util.Uint160, tokenID []byte) (util.Ui
// sends it to the network returning just a hash of it.
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) {
if !c.cache.initDone {
return util.Uint256{}, errNetworkNotInitialized
}
from, err := address.StringToUint160(acc.Address)
if err != nil {
return util.Uint256{}, fmt.Errorf("bad account address: %w", err)

8
pkg/rpc/client/nep17.go
View file

@ -142,10 +142,6 @@ func (c *Client) CreateTxFromScript(script []byte, acc *wallet.Account, sysFee,
// impossible (e.g. due to locked cosigner's account) an error is returned.
func (c *Client) TransferNEP17(acc *wallet.Account, to util.Uint160, token util.Uint160,
amount int64, gas int64, data interface{}, cosigners []SignerAccount) (util.Uint256, error) {
if !c.cache.initDone {
return util.Uint256{}, errNetworkNotInitialized
}
tx, err := c.CreateNEP17TransferTx(acc, to, token, amount, gas, data, cosigners)
if err != nil {
return util.Uint256{}, err
@ -156,10 +152,6 @@ func (c *Client) TransferNEP17(acc *wallet.Account, to util.Uint160, token util.
// MultiTransferNEP17 is similar to TransferNEP17, buf allows to have multiple recipients.
func (c *Client) MultiTransferNEP17(acc *wallet.Account, gas int64, recipients []TransferTarget, cosigners []SignerAccount) (util.Uint256, error) {
if !c.cache.initDone {
return util.Uint256{}, errNetworkNotInitialized
}
tx, err := c.CreateNEP17MultiTransferTx(acc, gas, recipients, cosigners)
if err != nil {
return util.Uint256{}, err

23
pkg/rpc/client/policy.go
View file

@ -10,25 +10,16 @@ import (
// GetFeePerByte invokes `getFeePerByte` method on a native Policy contract.
func (c *Client) GetFeePerByte() (int64, error) {
if !c.cache.initDone {
return 0, errNetworkNotInitialized
}
return c.invokeNativePolicyMethod("getFeePerByte")
}
// GetExecFeeFactor invokes `getExecFeeFactor` method on a native Policy contract.
func (c *Client) GetExecFeeFactor() (int64, error) {
if !c.cache.initDone {
return 0, errNetworkNotInitialized
}
return c.invokeNativePolicyMethod("getExecFeeFactor")
}
// GetStoragePrice invokes `getStoragePrice` method on a native Policy contract.
func (c *Client) GetStoragePrice() (int64, error) {
if !c.cache.initDone {
return 0, errNetworkNotInitialized
}
return c.invokeNativePolicyMethod("getStoragePrice")
}
@ -43,10 +34,11 @@ func (c *Client) GetMaxNotValidBeforeDelta() (int64, error) {
// invokeNativePolicy method invokes Get* method on a native Policy contract.
func (c *Client) invokeNativePolicyMethod(operation string) (int64, error) {
if !c.cache.initDone {
return 0, errNetworkNotInitialized
policyHash, err := c.GetNativeContractHash(nativenames.Policy)
if err != nil {
return 0, fmt.Errorf("failed to get native Policy hash: %w", err)
}
return c.invokeNativeGetMethod(c.cache.nativeHashes[nativenames.Policy], operation)
return c.invokeNativeGetMethod(policyHash, operation)
}
func (c *Client) invokeNativeGetMethod(hash util.Uint160, operation string) (int64, error) {
@ -63,10 +55,11 @@ func (c *Client) invokeNativeGetMethod(hash util.Uint160, operation string) (int
// IsBlocked invokes `isBlocked` method on native Policy contract.
func (c *Client) IsBlocked(hash util.Uint160) (bool, error) {
if !c.cache.initDone {
return false, errNetworkNotInitialized
policyHash, err := c.GetNativeContractHash(nativenames.Policy)
if err != nil {
return false, fmt.Errorf("failed to get native Policy hash: %w", err)
}
result, err := c.InvokeFunction(c.cache.nativeHashes[nativenames.Policy], "isBlocked", []smartcontract.Parameter{{
result, err := c.InvokeFunction(policyHash, "isBlocked", []smartcontract.Parameter{{
Type: smartcontract.Hash160Type,
Value: hash,
}}, nil)

69
pkg/rpc/client/rpc.go
View file

@ -94,14 +94,15 @@ func (c *Client) getBlock(params request.RawParams) (*block.Block, error) {
err error
b *block.Block
)
if !c.cache.initDone {
return nil, errNetworkNotInitialized
}
if err = c.performRequest("getblock", params, &resp); err != nil {
return nil, err
}
r := io.NewBinReaderFromBuf(resp)
b = block.New(c.StateRootInHeader())
sr, err := c.StateRootInHeader()
if err != nil {
return nil, err
}
b = block.New(sr)
b.DecodeBinary(r)
if r.Err != nil {
return nil, r.Err
@ -127,9 +128,11 @@ func (c *Client) getBlockVerbose(params request.RawParams) (*result.Block, error
resp = &result.Block{}
err error
)
if !c.cache.initDone {
return nil, errNetworkNotInitialized
sr, err := c.StateRootInHeader()
if err != nil {
return nil, err
}
resp.Header.StateRootEnabled = sr
if err = c.performRequest("getblock", params, resp); err != nil {
return nil, err
}
@ -157,14 +160,16 @@ func (c *Client) GetBlockHeader(hash util.Uint256) (*block.Header, error) {
resp []byte
h *block.Header
)
if !c.cache.initDone {
return nil, errNetworkNotInitialized
}
if err := c.performRequest("getblockheader", params, &resp); err != nil {
return nil, err
}
sr, err := c.StateRootInHeader()
if err != nil {
return nil, err
}
r := io.NewBinReaderFromBuf(resp)
h = new(block.Header)
h.StateRootEnabled = sr
h.DecodeBinary(r)
if r.Err != nil {
return nil, r.Err
@ -399,17 +404,13 @@ func (c *Client) GetRawMemPool() ([]util.Uint256, error) {
return *resp, nil
}
// GetRawTransaction returns a transaction by hash. You should initialize network magic
// with Init before calling GetRawTransaction.
// GetRawTransaction returns a transaction by hash.
func (c *Client) GetRawTransaction(hash util.Uint256) (*transaction.Transaction, error) {
var (
params = request.NewRawParams(hash.StringLE())
resp []byte
err error
)
if !c.cache.initDone {
return nil, errNetworkNotInitialized
}
if err = c.performRequest("getrawtransaction", params, &resp); err != nil {
return nil, err
}
@ -421,8 +422,7 @@ func (c *Client) GetRawTransaction(hash util.Uint256) (*transaction.Transaction,
}
// GetRawTransactionVerbose returns a transaction wrapper with additional
// metadata by transaction's hash. You should initialize network magic
// with Init before calling GetRawTransactionVerbose.
// metadata by transaction's hash.
// NOTE: to get transaction.ID and transaction.Size, use t.Hash() and io.GetVarSize(t) respectively.
func (c *Client) GetRawTransactionVerbose(hash util.Uint256) (*result.TransactionOutputRaw, error) {
var (
@ -430,9 +430,6 @@ func (c *Client) GetRawTransactionVerbose(hash util.Uint256) (*result.Transactio
resp = &result.TransactionOutputRaw{}
err error
)
if !c.cache.initDone {
return nil, errNetworkNotInitialized
}
if err = c.performRequest("getrawtransaction", params, resp); err != nil {
return nil, err
}
@ -687,7 +684,11 @@ func (c *Client) SignAndPushTx(tx *transaction.Transaction, acc *wallet.Account,
txHash util.Uint256
err error
)
if err = acc.SignTx(c.GetNetwork(), tx); err != nil {
m, err := c.GetNetwork()
if err != nil {
return txHash, fmt.Errorf("failed to sign tx: %w", err)
}
if err = acc.SignTx(m, tx); err != nil {
return txHash, fmt.Errorf("failed to sign tx: %w", err)
}
// try to add witnesses for the rest of the signers
@ -695,7 +696,7 @@ func (c *Client) SignAndPushTx(tx *transaction.Transaction, acc *wallet.Account,
var isOk bool
for _, cosigner := range cosigners {
if signer.Account == cosigner.Signer.Account {
err = cosigner.Account.SignTx(c.GetNetwork(), tx)
err = cosigner.Account.SignTx(m, tx)
if err != nil { // then account is non-contract-based and locked, but let's provide more detailed error
if paramNum := len(cosigner.Account.Contract.Parameters); paramNum != 0 && cosigner.Account.Contract.Deployed {
return txHash, fmt.Errorf("failed to add contract-based witness for signer #%d (%s): "+
@ -771,9 +772,6 @@ func getSigners(sender *wallet.Account, cosigners []SignerAccount) ([]transactio
// Note: client should be initialized before SignAndPushP2PNotaryRequest call.
func (c *Client) SignAndPushP2PNotaryRequest(mainTx *transaction.Transaction, fallbackScript []byte, fallbackSysFee int64, fallbackNetFee int64, fallbackValidFor uint32, acc *wallet.Account) (*payload.P2PNotaryRequest, error) {
var err error
if !c.cache.initDone {
return nil, errNetworkNotInitialized
}
notaryHash, err := c.GetNativeContractHash(nativenames.Notary)
if err != nil {
return nil, fmt.Errorf("failed to get native Notary hash: %w", err)
@ -835,7 +833,11 @@ func (c *Client) SignAndPushP2PNotaryRequest(mainTx *transaction.Transaction, fa
VerificationScript: []byte{},
},
}
if err = acc.SignTx(c.GetNetwork(), fallbackTx); err != nil {
m, err := c.GetNetwork()
if err != nil {
return nil, fmt.Errorf("failed to sign fallback tx: %w", err)
}
if err = acc.SignTx(m, fallbackTx); err != nil {
return nil, fmt.Errorf("failed to sign fallback tx: %w", err)
}
fallbackHash := fallbackTx.Hash()
@ -844,7 +846,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(c.GetNetwork()), req)...),
InvocationScript: append([]byte{byte(opcode.PUSHDATA1), 64}, acc.PrivateKey().SignHashable(uint32(m), req)...),
VerificationScript: acc.GetVerificationScript(),
}
actualHash, err := c.SubmitP2PNotaryRequest(req)
@ -990,13 +992,20 @@ func (c *Client) AddNetworkFee(tx *transaction.Transaction, extraFee int64, accs
}
// GetNetwork returns the network magic of the RPC node client connected to.
func (c *Client) GetNetwork() netmode.Magic {
return c.cache.network
func (c *Client) GetNetwork() (netmode.Magic, error) {
if !c.cache.initDone {
return 0, errNetworkNotInitialized
}
return c.cache.network, nil
}
// StateRootInHeader returns true if state root is contained in block header.
func (c *Client) StateRootInHeader() bool {
return c.cache.stateRootInHeader
// You should initialize Client cache with Init() before calling StateRootInHeader.
func (c *Client) StateRootInHeader() (bool, error) {
if !c.cache.initDone {
return false, errNetworkNotInitialized
}
return c.cache.stateRootInHeader, nil
}
// GetNativeContractHash returns native contract hash by its name.

8
pkg/rpc/client/rpc_test.go
View file

@ -6,6 +6,7 @@ import (
"encoding/base64"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"math/big"
"net/http"
@ -1913,7 +1914,8 @@ func TestGetNetwork(t *testing.T) {
t.Fatal(err)
}
// network was not initialised
require.Equal(t, netmode.Magic(0), c.GetNetwork())
_, err = c.GetNetwork()
require.True(t, errors.Is(err, errNetworkNotInitialized))
require.Equal(t, false, c.cache.initDone)
})
@ -1923,7 +1925,9 @@ func TestGetNetwork(t *testing.T) {
t.Fatal(err)
}
require.NoError(t, c.Init())
require.Equal(t, netmode.UnitTestNet, c.GetNetwork())
m, err := c.GetNetwork()
require.NoError(t, err)
require.Equal(t, netmode.UnitTestNet, m)
})
}

7
pkg/rpc/client/wsclient.go
View file

@ -141,7 +141,12 @@ readloop:
var val interface{}
switch event {
case response.BlockEventID:
val = block.New(c.StateRootInHeader())
sr, err := c.StateRootInHeader()
if err != nil {
// Client is not initialised.
break
}
val = block.New(sr)
case response.TransactionEventID:
val = &transaction.Transaction{}
case response.NotificationEventID:

1
pkg/rpc/client/wsclient_test.go
View file

@ -143,6 +143,7 @@ func TestWSClientEvents(t *testing.T) {
wsc, err := NewWS(context.TODO(), httpURLtoWS(srv.URL), Options{})
require.NoError(t, err)
wsc.cache.initDone = true // Our server mock is restricted, so perform initialisation manually.
wsc.cache.network = netmode.UnitTestNet
for range events {
select {

8
pkg/rpc/server/client_test.go
View file

@ -555,9 +555,11 @@ func TestSignAndPushP2PNotaryRequest(t *testing.T) {
c, err := client.New(context.Background(), httpSrv.URL, client.Options{})
require.NoError(t, err)
acc, err := wallet.NewAccount()
require.NoError(t, err)
t.Run("client wasn't initialized", func(t *testing.T) {
_, err := c.SignAndPushP2PNotaryRequest(nil, nil, 0, 0, 0, nil)
_, err := c.SignAndPushP2PNotaryRequest(transaction.New([]byte{byte(opcode.RET)}, 123), []byte{byte(opcode.RET)}, -1, 0, 100, acc)
require.NotNil(t, err)
})
@ -567,8 +569,6 @@ func TestSignAndPushP2PNotaryRequest(t *testing.T) {
require.NotNil(t, err)
})
acc, err := wallet.NewAccount()
require.NoError(t, err)
t.Run("bad fallback script", func(t *testing.T) {
_, err := c.SignAndPushP2PNotaryRequest(nil, []byte{byte(opcode.ASSERT)}, -1, 0, 0, acc)
require.NotNil(t, err)
@ -649,7 +649,7 @@ func TestCalculateNotaryFee(t *testing.T) {
t.Run("client not initialized", func(t *testing.T) {
_, err := c.CalculateNotaryFee(0)
require.NotNil(t, err)
require.NoError(t, err) // Do not require client initialisation for this.
})
}