From cfa542378d974c3628ba5f545e4a1610818bd24a Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Fri, 1 Dec 2023 17:42:08 +0100 Subject: [PATCH] chore(walletconnect)_: various improvements applied - `WalletConnectTransfer` identified as a new transfer type - Wallet-related endpoints that logically belong to the wallet moved from the wallet connect service - Wallet connect service now receives `transfer.TransactionManager` instead of `transactions.Transactor` - Deadlock issue when trying to send the tx with the wrong nonce fixed --- services/wallet/api.go | 47 ++++------ services/wallet/service.go | 2 +- .../wallet/transfer/transaction_manager.go | 58 +++++++++--- services/wallet/walletconnect/rpc.go | 60 ++----------- services/wallet/walletconnect/service.go | 90 ++++--------------- .../wallet/walletconnect/walletconnect.go | 9 -- transactions/pendingtxtracker.go | 1 + transactions/transactor.go | 40 ++++++--- 8 files changed, 115 insertions(+), 192 deletions(-) diff --git a/services/wallet/api.go b/services/wallet/api.go index c7a531d57..9dfeb5756 100644 --- a/services/wallet/api.go +++ b/services/wallet/api.go @@ -541,6 +541,23 @@ func (api *API) BuildTransaction(ctx context.Context, chainID uint64, sendTxArgs return api.s.transactionManager.BuildTransaction(chainID, params) } +func (api *API) BuildRawTransaction(ctx context.Context, chainID uint64, sendTxArgsJSON string, signature string) (response *transfer.TxResponse, err error) { + log.Debug("[WalletAPI::BuildRawTransaction]", "chainID", chainID, "sendTxArgsJSON", sendTxArgsJSON, "signature", signature) + + sig, err := hex.DecodeString(signature) + if err != nil { + return nil, err + } + + var params transactions.SendTxArgs + err = json.Unmarshal([]byte(sendTxArgsJSON), ¶ms) + if err != nil { + return nil, err + } + + return api.s.transactionManager.BuildRawTransaction(chainID, params, sig) +} + func (api *API) SendTransactionWithSignature(ctx context.Context, chainID uint64, txType transactions.PendingTrxType, sendTxArgsJSON string, signature string) (hash types.Hash, err error) { log.Debug("[WalletAPI::SendTransactionWithSignature]", "chainID", chainID, "txType", txType, "sendTxArgsJSON", sendTxArgsJSON, "signature", signature) @@ -641,34 +658,6 @@ func (api *API) FetchChainIDForURL(ctx context.Context, rpcURL string) (*big.Int return client.ChainID(ctx) } -// WCSignMessage signs a message for the passed address using the provided password and returns the signature -func (api *API) WCSignMessage(ctx context.Context, message types.HexBytes, address common.Address, password string) (string, error) { - log.Debug("wallet.api.wc.SignMessage", "message", message, "address", address, "password", password) - - return api.s.walletConnect.SignMessage(message, address, password) -} - -// WCBuildRawTransaction builds raw transaction using the provided signature and returns the RLP-encoded transaction object -func (api *API) WCBuildRawTransaction(signature string) (string, error) { - log.Debug("wallet.api.wc.BuildRawTransaction", "signature", signature) - - return api.s.walletConnect.BuildRawTransaction(signature) -} - -// WCSendRawTransaction sends provided raw transaction and returns the transaction hash -func (api *API) WCSendRawTransaction(rawTx string) (string, error) { - log.Debug("wallet.api.wc.SendRawTransaction", "rawTx", rawTx) - - return api.s.walletConnect.SendRawTransaction(rawTx) -} - -// WCSendTransactionWithSignature sends transaction with the provided signature and returns the transaction hash -func (api *API) WCSendTransactionWithSignature(signature string) (string, error) { - log.Debug("wallet.api.wc.SendTransactionWithSignature", "signature", signature) - - return api.s.walletConnect.SendTransactionWithSignature(signature) -} - // WCPairSessionProposal responds to "session_proposal" event func (api *API) WCPairSessionProposal(ctx context.Context, sessionProposalJSON string) (*wc.PairSessionResponse, error) { log.Debug("wallet.api.wc.PairSessionProposal", "proposal.len", len(sessionProposalJSON)) @@ -709,7 +698,7 @@ func (api *API) WCHasActivePairings(ctx context.Context) (bool, error) { } // WCSessionRequest responds to "session_request" event -func (api *API) WCSessionRequest(ctx context.Context, sessionRequestJSON string) (*wc.SessionRequestResponse, error) { +func (api *API) WCSessionRequest(ctx context.Context, sessionRequestJSON string) (*transfer.TxResponse, error) { log.Debug("wallet.api.wc.SessionRequest", "request.len", len(sessionRequestJSON)) var request wc.SessionRequest diff --git a/services/wallet/service.go b/services/wallet/service.go index 786344f03..c33f95ced 100644 --- a/services/wallet/service.go +++ b/services/wallet/service.go @@ -144,7 +144,7 @@ func NewService( activity := activity.NewService(db, tokenManager, collectiblesManager, feed) - walletconnect := walletconnect.NewService(db, rpcClient.NetworkManager, accountsDB, transactor, gethManager, feed, config) + walletconnect := walletconnect.NewService(db, rpcClient.NetworkManager, accountsDB, transactionManager, gethManager, feed, config) return &Service{ db: db, diff --git a/services/wallet/transfer/transaction_manager.go b/services/wallet/transfer/transaction_manager.go index 1ea936cdc..8220eacdd 100644 --- a/services/wallet/transfer/transaction_manager.go +++ b/services/wallet/transfer/transaction_manager.go @@ -132,8 +132,10 @@ type TxResponse struct { AddressPath string `json:"addressPath,omitempty"` SignOnKeycard bool `json:"signOnKeycard,omitempty"` ChainID uint64 `json:"chainId,omitempty"` - MesageToSign interface{} `json:"messageToSign,omitempty"` + MessageToSign interface{} `json:"messageToSign,omitempty"` TxArgs transactions.SendTxArgs `json:"txArgs,omitempty"` + RawTx string `json:"rawTx,omitempty"` + TxHash common.Hash `json:"txHash,omitempty"` } func (tm *TransactionManager) SignMessage(message types.HexBytes, address common.Address, password string) (string, error) { @@ -159,15 +161,15 @@ func (tm *TransactionManager) BuildTransaction(chainID uint64, sendArgs transact } txBeingSigned, unlock, err := tm.transactor.ValidateAndBuildTransaction(chainID, sendArgs) - if err != nil { - return nil, err - } // We have to unlock the nonce, cause we don't know what will happen on the client side (will user accept/reject) an action. if unlock != nil { defer func() { unlock(false, 0) }() } + if err != nil { + return nil, err + } // Set potential missing fields that were added while building the transaction if sendArgs.Value == nil { @@ -178,17 +180,24 @@ func (tm *TransactionManager) BuildTransaction(chainID uint64, sendArgs transact nonce := hexutil.Uint64(txBeingSigned.Nonce()) sendArgs.Nonce = &nonce } + if sendArgs.Gas == nil { + gas := hexutil.Uint64(txBeingSigned.Gas()) + sendArgs.Gas = &gas + } if sendArgs.GasPrice == nil { gasPrice := hexutil.Big(*txBeingSigned.GasPrice()) sendArgs.GasPrice = &gasPrice } - if sendArgs.MaxPriorityFeePerGas == nil { - maxPriorityFeePerGas := hexutil.Big(*txBeingSigned.GasTipCap()) - sendArgs.MaxPriorityFeePerGas = &maxPriorityFeePerGas - } - if sendArgs.MaxFeePerGas == nil { - maxFeePerGas := hexutil.Big(*txBeingSigned.GasFeeCap()) - sendArgs.MaxFeePerGas = &maxFeePerGas + + if sendArgs.IsDynamicFeeTx() { + if sendArgs.MaxPriorityFeePerGas == nil { + maxPriorityFeePerGas := hexutil.Big(*txBeingSigned.GasTipCap()) + sendArgs.MaxPriorityFeePerGas = &maxPriorityFeePerGas + } + if sendArgs.MaxFeePerGas == nil { + maxFeePerGas := hexutil.Big(*txBeingSigned.GasFeeCap()) + sendArgs.MaxFeePerGas = &maxFeePerGas + } } signer := ethTypes.NewLondonSigner(new(big.Int).SetUint64(chainID)) @@ -199,11 +208,36 @@ func (tm *TransactionManager) BuildTransaction(chainID uint64, sendArgs transact AddressPath: account.Path, SignOnKeycard: kp.MigratedToKeycard(), ChainID: chainID, - MesageToSign: signer.Hash(txBeingSigned), + MessageToSign: signer.Hash(txBeingSigned), TxArgs: sendArgs, }, nil } +func (tm *TransactionManager) BuildRawTransaction(chainID uint64, sendArgs transactions.SendTxArgs, signature []byte) (response *TxResponse, err error) { + tx, unlock, err := tm.transactor.BuildTransactionWithSignature(chainID, sendArgs, signature) + // We have to unlock the nonce, cause we don't know what will happen on the client side (will user accept/reject) an action. + if unlock != nil { + defer func() { + unlock(false, 0) + }() + } + if err != nil { + return nil, err + } + + data, err := tx.MarshalBinary() + if err != nil { + return nil, err + } + + return &TxResponse{ + ChainID: chainID, + TxArgs: sendArgs, + RawTx: types.EncodeHex(data), + TxHash: tx.Hash(), + }, nil +} + func (tm *TransactionManager) SendTransactionWithSignature(chainID uint64, txType transactions.PendingTrxType, sendArgs transactions.SendTxArgs, signature []byte) (hash types.Hash, err error) { hash, err = tm.transactor.BuildTransactionAndSendWithSignature(chainID, sendArgs, signature) if err != nil { diff --git a/services/wallet/walletconnect/rpc.go b/services/wallet/walletconnect/rpc.go index e5f7ecd9b..e17940ebb 100644 --- a/services/wallet/walletconnect/rpc.go +++ b/services/wallet/walletconnect/rpc.go @@ -1,18 +1,14 @@ package walletconnect import ( - "encoding/hex" "encoding/json" - "errors" "fmt" - "math/big" "strings" - "github.com/ethereum/go-ethereum/common" - ethTypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/signer/core/apitypes" "github.com/status-im/status-go/eth-node/crypto" "github.com/status-im/status-go/eth-node/types" + "github.com/status-im/status-go/services/wallet/transfer" "github.com/status-im/status-go/transactions" ) @@ -65,7 +61,7 @@ func (n *sendTransactionParams) MarshalJSON() ([]byte, error) { return json.Marshal(n.SendTxArgs) } -func (s *Service) buildTransaction(request SessionRequest) (response *SessionRequestResponse, err error) { +func (s *Service) buildTransaction(request SessionRequest) (response *transfer.TxResponse, err error) { if len(request.Params.Request.Params) != 1 { return nil, ErrorInvalidParamsCount } @@ -75,62 +71,16 @@ func (s *Service) buildTransaction(request SessionRequest) (response *SessionReq return nil, err } - account, err := s.accountsDB.GetAccountByAddress(params.From) - if err != nil { - return nil, fmt.Errorf("failed to get active account: %w", err) - } - - kp, err := s.accountsDB.GetKeypairByKeyUID(account.KeyUID) - if err != nil { - return nil, err - } - _, chainID, err := parseCaip2ChainID(request.Params.ChainID) if err != nil { return nil, err } - // In this case we can ignore `unlock` function received from `ValidateAndBuildTransaction` cause `Nonce` - // will be always set by the initiator of this transaction (by the dapp). - // Though we will need sort out completely that part since Nonce kept in the local cache is not the most recent one, - // instead of that we should always ask network what's the most recent known Nonce for the account. - // Logged issue to handle that: https://github.com/status-im/status-go/issues/4335 - txBeingSigned, _, err := s.transactor.ValidateAndBuildTransaction(chainID, params.SendTxArgs) - if err != nil { - return nil, err - } - - s.txSignDetails = &txSigningDetails{ - from: common.Address(account.Address), - chainID: chainID, - txBeingSigned: txBeingSigned, - } - - signer := ethTypes.NewLondonSigner(new(big.Int).SetUint64(chainID)) - return &SessionRequestResponse{ - KeyUID: account.KeyUID, - Address: account.Address, - AddressPath: account.Path, - SignOnKeycard: kp.MigratedToKeycard(), - MessageToSign: signer.Hash(txBeingSigned), - }, nil -} - -func (s *Service) addSignatureToTransaction(signature string) (*ethTypes.Transaction, error) { - if s.txSignDetails.txBeingSigned == nil { - return nil, errors.New("no tx to sign") - } - - signatureBytes, err := hex.DecodeString(signature) - if err != nil { - return nil, err - } - - return s.transactor.AddSignatureToTransaction(s.txSignDetails.chainID, s.txSignDetails.txBeingSigned, signatureBytes) + return s.transactionManager.BuildTransaction(chainID, params.SendTxArgs) } func (s *Service) buildMessage(request SessionRequest, addressIndex int, messageIndex int, - handleTypedData bool) (response *SessionRequestResponse, err error) { + handleTypedData bool) (response *transfer.TxResponse, err error) { if len(request.Params.Request.Params) != 2 { return nil, ErrorInvalidParamsCount } @@ -178,7 +128,7 @@ func (s *Service) buildMessage(request SessionRequest, addressIndex int, message } } - return &SessionRequestResponse{ + return &transfer.TxResponse{ KeyUID: account.KeyUID, Address: account.Address, AddressPath: account.Path, diff --git a/services/wallet/walletconnect/service.go b/services/wallet/walletconnect/service.go index bd1c19219..b2231c8a2 100644 --- a/services/wallet/walletconnect/service.go +++ b/services/wallet/walletconnect/service.go @@ -6,102 +6,42 @@ import ( "strings" "time" - "github.com/ethereum/go-ethereum/common" - ethTypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/log" "github.com/status-im/status-go/account" - "github.com/status-im/status-go/eth-node/crypto" - "github.com/status-im/status-go/eth-node/types" "github.com/status-im/status-go/multiaccounts/accounts" "github.com/status-im/status-go/params" "github.com/status-im/status-go/rpc/network" - "github.com/status-im/status-go/transactions" + "github.com/status-im/status-go/services/wallet/transfer" ) -type txSigningDetails struct { - chainID uint64 - from common.Address - txBeingSigned *ethTypes.Transaction - txHash common.Hash -} - type Service struct { db *sql.DB networkManager *network.Manager accountsDB *accounts.Database eventFeed *event.Feed - transactor *transactions.Transactor - gethManager *account.GethManager + transactionManager *transfer.TransactionManager + gethManager *account.GethManager - config *params.NodeConfig - txSignDetails *txSigningDetails + config *params.NodeConfig } -func NewService(db *sql.DB, networkManager *network.Manager, accountsDB *accounts.Database, transactor *transactions.Transactor, gethManager *account.GethManager, eventFeed *event.Feed, config *params.NodeConfig) *Service { +func NewService(db *sql.DB, networkManager *network.Manager, accountsDB *accounts.Database, + transactionManager *transfer.TransactionManager, gethManager *account.GethManager, eventFeed *event.Feed, + config *params.NodeConfig) *Service { return &Service{ - db: db, - networkManager: networkManager, - accountsDB: accountsDB, - eventFeed: eventFeed, - transactor: transactor, - gethManager: gethManager, - config: config, + db: db, + networkManager: networkManager, + accountsDB: accountsDB, + eventFeed: eventFeed, + transactionManager: transactionManager, + gethManager: gethManager, + config: config, } } -func (s *Service) SignMessage(message types.HexBytes, address common.Address, password string) (string, error) { - selectedAccount, err := s.gethManager.VerifyAccountPassword(s.config.KeyStoreDir, address.Hex(), password) - if err != nil { - return "", err - } - - signature, err := crypto.Sign(message[:], selectedAccount.PrivateKey) - - return types.EncodeHex(signature), err -} - -func (s *Service) BuildRawTransaction(signature string) (string, error) { - txWithSignature, err := s.addSignatureToTransaction(signature) - if err != nil { - return "", err - } - - data, err := txWithSignature.MarshalBinary() - if err != nil { - return "", err - } - - s.txSignDetails.txHash = txWithSignature.Hash() - - return types.EncodeHex(data), nil -} - -func (s *Service) SendRawTransaction(rawTx string) (string, error) { - err := s.transactor.SendRawTransaction(s.txSignDetails.chainID, rawTx) - if err != nil { - return "", err - } - - return s.txSignDetails.txHash.Hex(), nil -} - -func (s *Service) SendTransactionWithSignature(signature string) (string, error) { - txWithSignature, err := s.addSignatureToTransaction(signature) - if err != nil { - return "", err - } - - hash, err := s.transactor.SendTransactionWithSignature(txWithSignature) - if err != nil { - return "", err - } - - return hash.Hex(), nil -} - func (s *Service) PairSessionProposal(proposal SessionProposal) (*PairSessionResponse, error) { if !proposal.Valid() { return nil, ErrorInvalidSessionProposal @@ -208,7 +148,7 @@ func (s *Service) HasActivePairings() (bool, error) { return HasActivePairings(s.db, time.Now().Unix()) } -func (s *Service) SessionRequest(request SessionRequest) (response *SessionRequestResponse, err error) { +func (s *Service) SessionRequest(request SessionRequest) (response *transfer.TxResponse, err error) { // TODO #12434: should we check topic for validity? It might make sense if we // want to cache the paired sessions diff --git a/services/wallet/walletconnect/walletconnect.go b/services/wallet/walletconnect/walletconnect.go index 75fddcaf9..fcd926ca8 100644 --- a/services/wallet/walletconnect/walletconnect.go +++ b/services/wallet/walletconnect/walletconnect.go @@ -9,7 +9,6 @@ import ( "github.com/ethereum/go-ethereum/log" - "github.com/status-im/status-go/eth-node/types" "github.com/status-im/status-go/multiaccounts/accounts" "github.com/status-im/status-go/services/wallet/walletevent" ) @@ -102,14 +101,6 @@ type SessionDelete struct { Topic Topic `json:"topic"` } -type SessionRequestResponse struct { - KeyUID string `json:"keyUid,omitempty"` - Address types.Address `json:"address,omitempty"` - AddressPath string `json:"addressPath,omitempty"` - SignOnKeycard bool `json:"signOnKeycard,omitempty"` - MessageToSign interface{} `json:"messageToSign,omitempty"` -} - // Valid namespace func (n *Namespace) Valid(namespaceName string, chainID *uint64) bool { if chainID == nil { diff --git a/transactions/pendingtxtracker.go b/transactions/pendingtxtracker.go index 1aae84d05..feca43028 100644 --- a/transactions/pendingtxtracker.go +++ b/transactions/pendingtxtracker.go @@ -359,6 +359,7 @@ const ( BurnCommunityToken PendingTrxType = "BurnCommunityToken" DeployOwnerToken PendingTrxType = "DeployOwnerToken" SetSignerPublicKey PendingTrxType = "SetSignerPublicKey" + WalletConnectTransfer PendingTrxType = "WalletConnectTransfer" ) type PendingTransaction struct { diff --git a/transactions/transactor.go b/transactions/transactor.go index 39d36d076..78b361e56 100644 --- a/transactions/transactor.go +++ b/transactions/transactor.go @@ -167,32 +167,50 @@ func (t *Transactor) AddSignatureToTransactionAndSend(chainID uint64, tx *gethty // It's different from eth_sendRawTransaction because it receives a signature and not a serialized transaction with signature. // Since the transactions is already signed, we assume it was validated and used the right nonce. func (t *Transactor) BuildTransactionAndSendWithSignature(chainID uint64, args SendTxArgs, sig []byte) (hash types.Hash, err error) { + txWithSignature, unlock, err := t.BuildTransactionWithSignature(chainID, args, sig) + if unlock != nil { + defer func() { + var nonce uint64 + if txWithSignature != nil { + nonce = txWithSignature.Nonce() + } + unlock(err == nil, nonce) + }() + } + if err != nil { + return hash, err + } + + hash, err = t.SendTransactionWithSignature(txWithSignature) + return hash, err +} + +func (t *Transactor) BuildTransactionWithSignature(chainID uint64, args SendTxArgs, sig []byte) (*gethtypes.Transaction, UnlockNonceFunc, error) { if !args.Valid() { - return hash, ErrInvalidSendTxArgs + return nil, nil, ErrInvalidSendTxArgs } if len(sig) != ValidSignatureSize { - return hash, ErrInvalidSignatureSize + return nil, nil, ErrInvalidSignatureSize } tx := t.buildTransaction(args) rpcWrapper := newRPCWrapper(t.rpcWrapper.RPCClient, chainID) expectedNonce, unlock, err := t.nonce.Next(rpcWrapper, args.From) if err != nil { - return hash, err - } - if unlock != nil { - defer func() { - unlock(err == nil, expectedNonce) - }() + return nil, nil, err } if tx.Nonce() != expectedNonce { - return hash, &ErrBadNonce{tx.Nonce(), expectedNonce} + return nil, unlock, &ErrBadNonce{tx.Nonce(), expectedNonce} } - hash, err = t.AddSignatureToTransactionAndSend(chainID, tx, sig) - return hash, err + txWithSignature, err := t.AddSignatureToTransaction(chainID, tx, sig) + if err != nil { + return nil, unlock, err + } + + return txWithSignature, unlock, nil } func (t *Transactor) HashTransaction(args SendTxArgs) (validatedArgs SendTxArgs, hash types.Hash, err error) {