From 469001bc2906507f066b9d48c9bdf8a9e770ec1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Wed, 5 Oct 2022 09:49:37 +0300 Subject: [PATCH 01/12] Preparatory refactoring; changed validation logic; changed binding logic. --- server/services/constructionResources.go | 96 +++++++++++ server/services/constructionService.go | 196 +++++++++++----------- server/services/constructionServiceFee.go | 83 +++------ server/services/operations.go | 6 + 4 files changed, 219 insertions(+), 162 deletions(-) create mode 100644 server/services/constructionResources.go diff --git a/server/services/constructionResources.go b/server/services/constructionResources.go new file mode 100644 index 00000000..ab35764b --- /dev/null +++ b/server/services/constructionResources.go @@ -0,0 +1,96 @@ +package services + +import "encoding/json" + +type constructionPreprocessMetadata struct { + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Value string `json:"value"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` + Data []byte `json:"data"` +} + +func newConstructionPreprocessMetadata(obj objectsMap) (*constructionPreprocessMetadata, error) { + data, err := json.Marshal(obj) + if err != nil { + return nil, err + } + + result := &constructionPreprocessMetadata{} + err = json.Unmarshal(data, result) + if err != nil { + return nil, err + } + + return result, nil +} + +type constructionOptions struct { + FirstOperationType string `json:"type"` + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Value string `json:"value"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` + Data []byte `json:"data"` + MaxFee string `json:"maxFee"` + FeeMultiplier float64 `json:"feeMultiplier"` +} + +func newConstructionOptions(obj objectsMap) (*constructionOptions, error) { + data, err := json.Marshal(obj) + if err != nil { + return nil, err + } + + result := &constructionOptions{} + err = json.Unmarshal(data, result) + if err != nil { + return nil, err + } + + return result, nil +} + +func (options *constructionOptions) toObjectsMap() (objectsMap, error) { + data, err := json.Marshal(options) + if err != nil { + return nil, err + } + + var result objectsMap + err = json.Unmarshal(data, &result) + if err != nil { + return nil, err + } + + return result, nil +} + +type constructionMetadata struct { + Data []byte `json:"data"` + ChainID string `json:"chainID"` + Version int `json:"version"` + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Value string `json:"value"` + Nonce uint64 `json:"nonce"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` +} + +func (metadata *constructionMetadata) toObjectsMap() (objectsMap, error) { + data, err := json.Marshal(metadata) + if err != nil { + return nil, err + } + + var result objectsMap + err = json.Unmarshal(data, &result) + if err != nil { + return nil, err + } + + return result, nil +} diff --git a/server/services/constructionService.go b/server/services/constructionService.go index 28abe1d2..bf4cf5e3 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -5,7 +5,7 @@ import ( "encoding/hex" "encoding/json" "errors" - "fmt" + "strings" "github.com/ElrondNetwork/elrond-proxy-go/data" "github.com/coinbase/rosetta-sdk-go/server" @@ -34,44 +34,94 @@ func (service *constructionService) ConstructionPreprocess( _ context.Context, request *types.ConstructionPreprocessRequest, ) (*types.ConstructionPreprocessResponse, *types.Error) { + log.Info("constructionService.ConstructionPreprocess()", + "metadata", request.Metadata, + "maxFee", request.MaxFee, + "suggestedFeeMultiplier", request.SuggestedFeeMultiplier, + ) + + requestMetadata, err := newConstructionPreprocessMetadata(request.Metadata) + if err != nil { + return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) + } + + responseOptions := &constructionOptions{} + + // TODO: if err := service.checkOperationsAndMeta(request.Operations, request.Metadata); err != nil { return nil, err } - options, err := service.getOptionsFromOperations(request.Operations) - if err != nil { - return nil, err + if len(requestMetadata.Sender) > 0 { + responseOptions.Sender = requestMetadata.Sender + } else { + responseOptions.Sender = request.Operations[0].Account.Address } + if len(requestMetadata.Receiver) > 0 { + responseOptions.Receiver = requestMetadata.Receiver + } else { + if len(request.Operations) < 2 { + return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("cannot prepare receiver")) + } + responseOptions.Receiver = request.Operations[1].Account.Address + } + + if len(requestMetadata.Value) > 0 { + responseOptions.Value = requestMetadata.Value + } else { + // TODO: + responseOptions.Value = strings.Trim(request.Operations[0].Amount.Value, "-") + } + + responseOptions.FirstOperationType = request.Operations[0].Type + if len(request.MaxFee) > 0 { + // TODO: maxFee := request.MaxFee[0] if !service.extension.isNativeCurrency(maxFee.Currency) { return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid currency")) } - options["maxFee"] = maxFee.Value + responseOptions.MaxFee = maxFee.Value } if request.SuggestedFeeMultiplier != nil { - options["feeMultiplier"] = *request.SuggestedFeeMultiplier + responseOptions.FeeMultiplier = *request.SuggestedFeeMultiplier } - - if request.Metadata["gasLimit"] != nil { - options["gasLimit"] = request.Metadata["gasLimit"] + if requestMetadata.GasLimit > 0 { + responseOptions.GasLimit = requestMetadata.GasLimit + } + if requestMetadata.GasPrice > 0 { + responseOptions.GasPrice = requestMetadata.GasPrice } - if request.Metadata["gasPrice"] != nil { - options["gasPrice"] = request.Metadata["gasPrice"] + if len(requestMetadata.Data) > 0 { + responseOptions.Data = requestMetadata.Data } - if request.Metadata["data"] != nil { - options["data"] = request.Metadata["data"] + + optionsAsObjectsMap, err := responseOptions.toObjectsMap() + if err != nil { + return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) } + // TODO: Ensure sender & receiver & value & FirstOperationType are known! + // if metadata["sender"], ok = options["sender"]; !ok { + // return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("sender address missing")) + // } + // if metadata["receiver"], ok = options["receiver"]; !ok { + // return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("receiver address missing")) + // } + // if metadata["value"], ok = options["value"]; !ok { + // return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("value missing")) + // } + return &types.ConstructionPreprocessResponse{ - Options: options, + Options: optionsAsObjectsMap, }, nil } func (service *constructionService) checkOperationsAndMeta(ops []*types.Operation, meta map[string]interface{}) *types.Error { + // TODO: remove this constraint; metadata should be sufficient, as well if len(ops) == 0 { return service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid number of operations")) } @@ -85,31 +135,11 @@ func (service *constructionService) checkOperationsAndMeta(ops []*types.Operatio } } - if meta["gasLimit"] != nil { - if !checkValueIsOk(meta["gasLimit"]) { - return service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid metadata gas limit")) - } - } - if meta["gasPrice"] != nil { - if !checkValueIsOk(meta["gasPrice"]) { - return service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid metadata gas price")) - } - } - return nil } -func checkValueIsOk(value interface{}) bool { - switch value.(type) { - case uint64, float64, int: - return true - default: - return false - } -} - func checkOperationsType(op *types.Operation) bool { - for _, supOp := range SupportedOperationTypes { + for _, supOp := range SupportedOperationTypesForConstruction { if supOp == op.Type { return true } @@ -118,100 +148,68 @@ func checkOperationsType(op *types.Operation) bool { return false } -func (service *constructionService) getOptionsFromOperations(ops []*types.Operation) (objectsMap, *types.Error) { - if len(ops) < 2 { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid number of operations")) - } - options := make(objectsMap) - options["sender"] = ops[0].Account.Address - options["receiver"] = ops[1].Account.Address - options["type"] = ops[0].Type - options["value"] = ops[1].Amount.Value - - return options, nil -} - // ConstructionMetadata gets any information required to construct a transaction for a specific network (e.g. the account nonce) func (service *constructionService) ConstructionMetadata( _ context.Context, request *types.ConstructionMetadataRequest, ) (*types.ConstructionMetadataResponse, *types.Error) { + log.Info("constructionService.ConstructionMetadata()", "options", request.Options) + if service.provider.IsOffline() { return nil, service.errFactory.newErr(ErrOfflineMode) } - txType, ok := request.Options["type"].(string) - if !ok { - return nil, service.errFactory.newErrWithOriginal(ErrInvalidInputParam, errors.New("invalid operation type")) + requestOptions, err := newConstructionOptions(request.Options) + if err != nil { + return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) } - metadata, err := service.computeMetadata(request.Options) + txType := requestOptions.FirstOperationType + + metadata := &constructionMetadata{} + metadata.Data = requestOptions.Data + metadata.ChainID = service.provider.GetNetworkConfig().NetworkID + metadata.Version = transactionVersion + metadata.Sender = requestOptions.Sender + metadata.Receiver = requestOptions.Receiver + metadata.Value = requestOptions.Value + + account, err := service.provider.GetAccount(requestOptions.Sender) if err != nil { - return nil, err + return nil, service.errFactory.newErrWithOriginal(ErrUnableToGetAccount, err) } + metadata.Nonce = account.Account.Nonce + networkConfig := service.provider.GetNetworkConfig() - suggestedFee, gasPrice, gasLimit, err := service.computeSuggestedFeeAndGas(txType, request.Options, networkConfig) + suggestedFee, gasPrice, gasLimit, errTyped := service.computeSuggestedFeeAndGas(txType, requestOptions, networkConfig) if err != nil { - return nil, err + return nil, errTyped } - metadata["gasLimit"] = gasLimit - metadata["gasPrice"] = gasPrice + metadata.GasLimit = gasLimit + metadata.GasPrice = gasPrice + + metadataAsObjectsMap, err := metadata.toObjectsMap() + if err != nil { + return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) + } return &types.ConstructionMetadataResponse{ - Metadata: metadata, + Metadata: metadataAsObjectsMap, SuggestedFee: []*types.Amount{ service.extension.valueToNativeAmount(suggestedFee.String()), }, }, nil } -func (service *constructionService) computeMetadata(options objectsMap) (objectsMap, *types.Error) { - metadata := make(objectsMap) - if dataField, ok := options["data"]; ok { - // convert string to byte array - metadata["data"] = []byte(fmt.Sprintf("%v", dataField)) - } - - var ok bool - if metadata["sender"], ok = options["sender"]; !ok { - return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("sender address missing")) - } - if metadata["receiver"], ok = options["receiver"]; !ok { - return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("receiver address missing")) - } - if metadata["value"], ok = options["value"]; !ok { - return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("value missing")) - } - - metadata["chainID"] = service.provider.GetNetworkConfig().NetworkID - metadata["version"] = transactionVersion - - senderAddressI, ok := options["sender"] - if !ok { - return nil, service.errFactory.newErrWithOriginal(ErrInvalidInputParam, errors.New("cannot find sender address")) - } - senderAddress, ok := senderAddressI.(string) - if !ok { - return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("sender address is invalid")) - } - - account, err := service.provider.GetAccount(senderAddress) - if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrUnableToGetAccount, err) - } - - metadata["nonce"] = account.Account.Nonce - - return metadata, nil -} - // ConstructionPayloads returns an unsigned transaction blob and a collection of payloads that must be signed func (service *constructionService) ConstructionPayloads( _ context.Context, request *types.ConstructionPayloadsRequest, ) (*types.ConstructionPayloadsResponse, *types.Error) { + log.Info("constructionService.ConstructionPayloads()", "metadata", request.Metadata) + if err := service.checkOperationsAndMeta(request.Operations, request.Metadata); err != nil { return nil, err } @@ -384,6 +382,8 @@ func (service *constructionService) ConstructionSubmit( _ context.Context, request *types.ConstructionSubmitRequest, ) (*types.TransactionIdentifierResponse, *types.Error) { + log.Info("constructionService.ConstructionSubmit()", "transaction", request.SignedTransaction) + if service.provider.IsOffline() { return nil, service.errFactory.newErr(ErrOfflineMode) } diff --git a/server/services/constructionServiceFee.go b/server/services/constructionServiceFee.go index e7265cda..db0706db 100644 --- a/server/services/constructionServiceFee.go +++ b/server/services/constructionServiceFee.go @@ -1,24 +1,21 @@ package services import ( - "fmt" "math/big" "github.com/ElrondNetwork/rosetta/server/resources" "github.com/coinbase/rosetta-sdk-go/types" ) -func (service *constructionService) computeSuggestedFeeAndGas(txType string, options objectsMap, networkConfig *resources.NetworkConfig) (*big.Int, uint64, uint64, *types.Error) { - var gasLimit, gasPrice uint64 - - if gasLimitI, ok := options["gasLimit"]; ok { - gasLimit = getUint64Value(gasLimitI) +func (service *constructionService) computeSuggestedFeeAndGas(txType string, options *constructionOptions, networkConfig *resources.NetworkConfig) (*big.Int, uint64, uint64, *types.Error) { + gasLimit := options.GasLimit + gasPrice := options.GasPrice + if gasLimit > 0 { err := service.checkProvidedGasLimit(gasLimit, txType, options, networkConfig) if err != nil { return nil, 0, 0, err } - } else { // if gas limit is not provided, we estimate it estimatedGasLimit, err := service.estimateGasLimit(txType, networkConfig, options) @@ -29,66 +26,35 @@ func (service *constructionService) computeSuggestedFeeAndGas(txType string, opt gasLimit = estimatedGasLimit } - if gasPriceI, ok := options["gasPrice"]; ok { - gasPrice = getUint64Value(gasPriceI) - + if gasPrice > 0 { if gasPrice < networkConfig.MinGasPrice { return nil, 0, 0, service.errFactory.newErr(ErrGasPriceTooLow) } - } else { // if gas price is not provided, we set it to minGasPrice gasPrice = networkConfig.MinGasPrice } - suggestedFee := big.NewInt(0).Mul( - big.NewInt(0).SetUint64(gasPrice), - big.NewInt(0).SetUint64(gasLimit), - ) - - suggestedFee, gasPrice = service.adjustTxFeeWithFeeMultiplier(suggestedFee, gasPrice, options, networkConfig.MinGasPrice) - - return suggestedFee, gasPrice, gasLimit, nil -} - -func (service *constructionService) adjustTxFeeWithFeeMultiplier( - txFee *big.Int, gasPrice uint64, options objectsMap, minGasPrice uint64, -) (*big.Int, uint64) { - feeMultiplierI, ok := options["feeMultiplier"] - if !ok { - return txFee, gasPrice - } - - feeMultiplier, ok := feeMultiplierI.(float64) - if !ok { - return txFee, gasPrice - } + if options.FeeMultiplier != 0 { + gasPrice = uint64(options.FeeMultiplier * float64(gasPrice)) - feeMultiplierBig := big.NewFloat(feeMultiplier) - bigVal, ok := big.NewFloat(0).SetString(txFee.String()) - if !ok { - return txFee, gasPrice + if gasPrice < networkConfig.MinGasPrice { + gasPrice = networkConfig.MinGasPrice + } } - bigVal.Mul(bigVal, feeMultiplierBig) - - result := new(big.Int) - bigVal.Int(result) + fee := big.NewInt(0).Mul( + big.NewInt(0).SetUint64(gasPrice), + big.NewInt(0).SetUint64(gasLimit), + ) - gasPrice = uint64(feeMultiplier * float64(gasPrice)) - if gasPrice < minGasPrice { - gasPrice = minGasPrice - } + // TODO: In the case that the caller provides both a max fee and a suggested fee multiplier, the max fee will set an upper bound on the suggested fee (regardless of the multiplier provided). - return result, gasPrice + return fee, gasPrice, gasLimit, nil } -func (service *constructionService) estimateGasLimit(operationType string, networkConfig *resources.NetworkConfig, options objectsMap) (uint64, *types.Error) { - gasForDataField := uint64(0) - if dataFieldI, ok := options["data"]; ok { - dataField := fmt.Sprintf("%v", dataFieldI) - gasForDataField = networkConfig.GasPerDataByte * uint64(len(dataField)) - } +func (service *constructionService) estimateGasLimit(operationType string, networkConfig *resources.NetworkConfig, options *constructionOptions) (uint64, *types.Error) { + gasForDataField := networkConfig.GasPerDataByte * uint64(len(options.Data)) switch operationType { case opTransfer: @@ -99,7 +65,7 @@ func (service *constructionService) estimateGasLimit(operationType string, netwo } } -func (service *constructionService) checkProvidedGasLimit(providedGasLimit uint64, txType string, options objectsMap, networkConfig *resources.NetworkConfig) *types.Error { +func (service *constructionService) checkProvidedGasLimit(providedGasLimit uint64, txType string, options *constructionOptions, networkConfig *resources.NetworkConfig) *types.Error { estimatedGasLimit, err := service.estimateGasLimit(txType, networkConfig, options) if err != nil { return err @@ -111,14 +77,3 @@ func (service *constructionService) checkProvidedGasLimit(providedGasLimit uint6 return nil } - -func getUint64Value(obj interface{}) uint64 { - if value, ok := obj.(uint64); ok { - return value - } - if value, ok := obj.(float64); ok { - return uint64(value) - } - - return 0 -} diff --git a/server/services/operations.go b/server/services/operations.go index 1d0b2dc3..4135dcae 100644 --- a/server/services/operations.go +++ b/server/services/operations.go @@ -28,6 +28,12 @@ var ( opFeeRefund, } + // SupportedOperationTypesForConstruction is a list of the supported operations for transaction construction + SupportedOperationTypesForConstruction = []string{ + opTransfer, + opFee, + } + opStatusSuccess = "Success" opStatusFailure = "Failure" From 474bcd1611b9aad41cee8e10e0e520c9c317f93a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Fri, 7 Oct 2022 16:09:50 +0300 Subject: [PATCH 02/12] Continue re-design. Unit test are failing. Work in progress. --- server/services/common.go | 57 ++++++ server/services/constructionMetadata.go | 88 +++++++++ server/services/constructionOptions.go | 57 ++++++ .../constructionPreprocessMetadata.go | 21 +++ server/services/constructionResources.go | 96 ---------- server/services/constructionService.go | 173 +++++++----------- server/services/constructionServiceFee.go | 105 +++++------ server/services/errors.go | 6 +- server/services/networkProviderExtension.go | 8 + server/services/operations.go | 6 - 10 files changed, 352 insertions(+), 265 deletions(-) create mode 100644 server/services/constructionMetadata.go create mode 100644 server/services/constructionOptions.go create mode 100644 server/services/constructionPreprocessMetadata.go delete mode 100644 server/services/constructionResources.go diff --git a/server/services/common.go b/server/services/common.go index 027e8f11..8a257c96 100644 --- a/server/services/common.go +++ b/server/services/common.go @@ -1,3 +1,60 @@ package services +import ( + "encoding/json" + "math/big" + "strings" +) + type objectsMap map[string]interface{} + +func toObjectsMap(value any) (objectsMap, error) { + data, err := json.Marshal(value) + if err != nil { + return nil, err + } + + var result objectsMap + err = json.Unmarshal(data, &result) + if err != nil { + return nil, err + } + + return result, nil +} + +func fromObjectsMap(obj objectsMap, value any) error { + data, err := json.Marshal(obj) + if err != nil { + return err + } + + err = json.Unmarshal(data, value) + if err != nil { + return err + } + + return nil +} + +func isZeroAmount(amount string) bool { + return amount == "" || amount == "0" || amount == "-0" +} + +func getMagnitudeOfAmount(amount string) string { + return strings.Trim(amount, "-") +} + +func multiplyUint64(a uint64, b uint64) *big.Int { + return big.NewInt(0).Mul(big.NewInt(0).SetUint64(a), big.NewInt(0).SetUint64(b)) +} + +func divideBigIntByBigFloat(numerator *big.Int, denominator *big.Float) *big.Float { + result := big.NewFloat(0).SetInt(numerator) + result = result.Quo(result, denominator) + return result +} + +func addBigInt(a *big.Int, b *big.Int) *big.Int { + return big.NewInt(0).And(a, b) +} diff --git a/server/services/constructionMetadata.go b/server/services/constructionMetadata.go new file mode 100644 index 00000000..5281a472 --- /dev/null +++ b/server/services/constructionMetadata.go @@ -0,0 +1,88 @@ +package services + +import ( + "encoding/json" + "errors" + + "github.com/ElrondNetwork/elrond-proxy-go/data" +) + +type constructionMetadata struct { + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Nonce uint64 `json:"nonce"` + Amount string `json:"amount"` + CurrencySymbol string `json:"currencySymbol"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` + Data []byte `json:"data"` + ChainID string `json:"chainID"` + Version int `json:"version"` +} + +func newConstructionMetadata(obj objectsMap) (*constructionMetadata, error) { + result := &constructionMetadata{} + err := fromObjectsMap(obj, result) + if err != nil { + return nil, err + } + + return result, nil +} + +func (metadata *constructionMetadata) toTransactionJson() ([]byte, error) { + tx, err := metadata.toTransaction() + if err != nil { + return nil, err + } + + txJson, err := json.Marshal(tx) + if err != nil { + return nil, err + } + + return txJson, nil +} + +func (metadata *constructionMetadata) toTransaction() (*data.Transaction, error) { + err := metadata.validate() + if err != nil { + return nil, err + } + + tx := &data.Transaction{} + tx.Sender = metadata.Sender + tx.Receiver = metadata.Receiver + tx.Nonce = metadata.Nonce + tx.Value = metadata.Amount + tx.GasLimit = metadata.GasLimit + tx.GasPrice = metadata.GasPrice + tx.Data = nil + tx.ChainID = metadata.ChainID + tx.Version = uint32(metadata.Version) + + return tx, nil +} + +func (metadata *constructionMetadata) validate() error { + if len(metadata.Sender) == 0 { + return errors.New("missing metadata: sender") + } + if len(metadata.Receiver) == 0 { + return errors.New("missing metadata: receiver") + } + if metadata.GasLimit == 0 { + return errors.New("missing metadata: gasLimit") + } + if metadata.GasPrice == 0 { + return errors.New("missing metadata: gasPrice") + } + if metadata.Version != 1 { + return errors.New("bad metadata: unexpected version") + } + if len(metadata.ChainID) == 0 { + return errors.New("missing metadata: chainID") + } + + return nil +} diff --git a/server/services/constructionOptions.go b/server/services/constructionOptions.go new file mode 100644 index 00000000..4529b651 --- /dev/null +++ b/server/services/constructionOptions.go @@ -0,0 +1,57 @@ +package services + +import ( + "errors" + "math/big" +) + +type constructionOptions struct { + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Amount string `json:"amount"` + CurrencySymbol string `json:"currencySymbol"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` + Data []byte `json:"data"` + MaxFee string `json:"maxFee"` + FeeMultiplier float64 `json:"feeMultiplier"` +} + +func newConstructionOptions(obj objectsMap) (*constructionOptions, error) { + result := &constructionOptions{} + err := fromObjectsMap(obj, result) + if err != nil { + return nil, err + } + + return result, nil +} + +func (options *constructionOptions) getMaxFee() *big.Int { + maxFee, ok := big.NewInt(0).SetString(options.MaxFee, 10) + if ok { + return maxFee + } + + return big.NewInt(0) +} + +func (options *constructionOptions) validate(nativeCurrencySymbol string) error { + if len(options.Sender) == 0 { + return errors.New("missing option 'sender'") + } + if len(options.Receiver) == 0 { + return errors.New("missing option 'receive'") + } + if isZeroAmount(options.Amount) { + return errors.New("missing option 'amount'") + } + if len(options.CurrencySymbol) == 0 { + return errors.New("missing option 'currencySymbol'") + } + if len(options.Data) > 0 && options.CurrencySymbol != nativeCurrencySymbol { + return errors.New("for custom currencies, cannot populate option 'data'") + } + + return nil +} diff --git a/server/services/constructionPreprocessMetadata.go b/server/services/constructionPreprocessMetadata.go new file mode 100644 index 00000000..613b11f9 --- /dev/null +++ b/server/services/constructionPreprocessMetadata.go @@ -0,0 +1,21 @@ +package services + +type constructionPreprocessMetadata struct { + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Amount string `json:"amount"` + CurrencySymbol string `json:"currencySymbol"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` + Data []byte `json:"data"` +} + +func newConstructionPreprocessMetadata(obj objectsMap) (*constructionPreprocessMetadata, error) { + result := &constructionPreprocessMetadata{} + err := fromObjectsMap(obj, result) + if err != nil { + return nil, err + } + + return result, nil +} diff --git a/server/services/constructionResources.go b/server/services/constructionResources.go deleted file mode 100644 index ab35764b..00000000 --- a/server/services/constructionResources.go +++ /dev/null @@ -1,96 +0,0 @@ -package services - -import "encoding/json" - -type constructionPreprocessMetadata struct { - Sender string `json:"sender"` - Receiver string `json:"receiver"` - Value string `json:"value"` - GasLimit uint64 `json:"gasLimit"` - GasPrice uint64 `json:"gasPrice"` - Data []byte `json:"data"` -} - -func newConstructionPreprocessMetadata(obj objectsMap) (*constructionPreprocessMetadata, error) { - data, err := json.Marshal(obj) - if err != nil { - return nil, err - } - - result := &constructionPreprocessMetadata{} - err = json.Unmarshal(data, result) - if err != nil { - return nil, err - } - - return result, nil -} - -type constructionOptions struct { - FirstOperationType string `json:"type"` - Sender string `json:"sender"` - Receiver string `json:"receiver"` - Value string `json:"value"` - GasLimit uint64 `json:"gasLimit"` - GasPrice uint64 `json:"gasPrice"` - Data []byte `json:"data"` - MaxFee string `json:"maxFee"` - FeeMultiplier float64 `json:"feeMultiplier"` -} - -func newConstructionOptions(obj objectsMap) (*constructionOptions, error) { - data, err := json.Marshal(obj) - if err != nil { - return nil, err - } - - result := &constructionOptions{} - err = json.Unmarshal(data, result) - if err != nil { - return nil, err - } - - return result, nil -} - -func (options *constructionOptions) toObjectsMap() (objectsMap, error) { - data, err := json.Marshal(options) - if err != nil { - return nil, err - } - - var result objectsMap - err = json.Unmarshal(data, &result) - if err != nil { - return nil, err - } - - return result, nil -} - -type constructionMetadata struct { - Data []byte `json:"data"` - ChainID string `json:"chainID"` - Version int `json:"version"` - Sender string `json:"sender"` - Receiver string `json:"receiver"` - Value string `json:"value"` - Nonce uint64 `json:"nonce"` - GasLimit uint64 `json:"gasLimit"` - GasPrice uint64 `json:"gasPrice"` -} - -func (metadata *constructionMetadata) toObjectsMap() (objectsMap, error) { - data, err := json.Marshal(metadata) - if err != nil { - return nil, err - } - - var result objectsMap - err = json.Unmarshal(data, &result) - if err != nil { - return nil, err - } - - return result, nil -} diff --git a/server/services/constructionService.go b/server/services/constructionService.go index bf4cf5e3..8a05a33e 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -5,7 +5,6 @@ import ( "encoding/hex" "encoding/json" "errors" - "strings" "github.com/ElrondNetwork/elrond-proxy-go/data" "github.com/coinbase/rosetta-sdk-go/server" @@ -34,53 +33,66 @@ func (service *constructionService) ConstructionPreprocess( _ context.Context, request *types.ConstructionPreprocessRequest, ) (*types.ConstructionPreprocessResponse, *types.Error) { - log.Info("constructionService.ConstructionPreprocess()", + log.Debug("constructionService.ConstructionPreprocess()", "metadata", request.Metadata, "maxFee", request.MaxFee, "suggestedFeeMultiplier", request.SuggestedFeeMultiplier, ) + noOperationProvided := len(request.Operations) == 0 + lessThanTwoOperationsProvided := len(request.Operations) < 2 + requestMetadata, err := newConstructionPreprocessMetadata(request.Metadata) if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) } responseOptions := &constructionOptions{} - // TODO: - if err := service.checkOperationsAndMeta(request.Operations, request.Metadata); err != nil { - return nil, err - } - if len(requestMetadata.Sender) > 0 { responseOptions.Sender = requestMetadata.Sender } else { + // Fallback: get "sender" from the first operation + if noOperationProvided { + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, errors.New("cannot prepare sender")) + } responseOptions.Sender = request.Operations[0].Account.Address } if len(requestMetadata.Receiver) > 0 { responseOptions.Receiver = requestMetadata.Receiver } else { - if len(request.Operations) < 2 { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("cannot prepare receiver")) + // Fallback: get "receiver" from the second operation + if lessThanTwoOperationsProvided { + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, errors.New("cannot prepare receiver")) } responseOptions.Receiver = request.Operations[1].Account.Address } - if len(requestMetadata.Value) > 0 { - responseOptions.Value = requestMetadata.Value + if len(requestMetadata.Amount) > 0 { + responseOptions.Amount = requestMetadata.Amount } else { - // TODO: - responseOptions.Value = strings.Trim(request.Operations[0].Amount.Value, "-") + // Fallback: get "amount" from the first operation + if noOperationProvided { + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, errors.New("cannot prepare amount")) + } + responseOptions.Amount = getMagnitudeOfAmount(request.Operations[0].Amount.Value) } - responseOptions.FirstOperationType = request.Operations[0].Type + if len(requestMetadata.CurrencySymbol) > 0 { + responseOptions.CurrencySymbol = requestMetadata.CurrencySymbol + } else { + // Fallback: get "currencySymbol" from the first operation + if noOperationProvided { + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, errors.New("cannot prepare currency")) + } + responseOptions.CurrencySymbol = request.Operations[0].Amount.Currency.Symbol + } if len(request.MaxFee) > 0 { - // TODO: maxFee := request.MaxFee[0] if !service.extension.isNativeCurrency(maxFee.Currency) { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid currency")) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, errors.New("invalid currency for fee")) } responseOptions.MaxFee = maxFee.Value @@ -99,61 +111,29 @@ func (service *constructionService) ConstructionPreprocess( responseOptions.Data = requestMetadata.Data } - optionsAsObjectsMap, err := responseOptions.toObjectsMap() + err = responseOptions.validate( + service.extension.getNativeCurrencySymbol(), + ) if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) } - // TODO: Ensure sender & receiver & value & FirstOperationType are known! - // if metadata["sender"], ok = options["sender"]; !ok { - // return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("sender address missing")) - // } - // if metadata["receiver"], ok = options["receiver"]; !ok { - // return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("receiver address missing")) - // } - // if metadata["value"], ok = options["value"]; !ok { - // return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, errors.New("value missing")) - // } + optionsAsObjectsMap, err := toObjectsMap(responseOptions) + if err != nil { + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) + } return &types.ConstructionPreprocessResponse{ Options: optionsAsObjectsMap, }, nil } -func (service *constructionService) checkOperationsAndMeta(ops []*types.Operation, meta map[string]interface{}) *types.Error { - // TODO: remove this constraint; metadata should be sufficient, as well - if len(ops) == 0 { - return service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("invalid number of operations")) - } - - for _, op := range ops { - if !checkOperationsType(op) { - return service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("unsupported operation type")) - } - if op.Amount.Currency.Symbol != service.extension.getNativeCurrency().Symbol { - return service.errFactory.newErrWithOriginal(ErrConstructionCheck, errors.New("unsupported currency symbol")) - } - } - - return nil -} - -func checkOperationsType(op *types.Operation) bool { - for _, supOp := range SupportedOperationTypesForConstruction { - if supOp == op.Type { - return true - } - } - - return false -} - // ConstructionMetadata gets any information required to construct a transaction for a specific network (e.g. the account nonce) func (service *constructionService) ConstructionMetadata( _ context.Context, request *types.ConstructionMetadataRequest, ) (*types.ConstructionMetadataResponse, *types.Error) { - log.Info("constructionService.ConstructionMetadata()", "options", request.Options) + log.Debug("constructionService.ConstructionMetadata()", "options", request.Options) if service.provider.IsOffline() { return nil, service.errFactory.newErr(ErrOfflineMode) @@ -161,38 +141,38 @@ func (service *constructionService) ConstructionMetadata( requestOptions, err := newConstructionOptions(request.Options) if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) } - txType := requestOptions.FirstOperationType - - metadata := &constructionMetadata{} - metadata.Data = requestOptions.Data - metadata.ChainID = service.provider.GetNetworkConfig().NetworkID - metadata.Version = transactionVersion - metadata.Sender = requestOptions.Sender - metadata.Receiver = requestOptions.Receiver - metadata.Value = requestOptions.Value - account, err := service.provider.GetAccount(requestOptions.Sender) if err != nil { return nil, service.errFactory.newErrWithOriginal(ErrUnableToGetAccount, err) } - metadata.Nonce = account.Account.Nonce + computedData := service.computeData(requestOptions) - networkConfig := service.provider.GetNetworkConfig() - suggestedFee, gasPrice, gasLimit, errTyped := service.computeSuggestedFeeAndGas(txType, requestOptions, networkConfig) + suggestedFee, gasPrice, gasLimit, errTyped := service.computeSuggestedFeeAndGas(requestOptions, computedData) if err != nil { return nil, errTyped } + // + + metadata := &constructionMetadata{} + metadata.Nonce = account.Account.Nonce + metadata.Sender = requestOptions.Sender + metadata.Receiver = requestOptions.Receiver + metadata.Amount = requestOptions.Amount + metadata.CurrencySymbol = requestOptions.CurrencySymbol metadata.GasLimit = gasLimit metadata.GasPrice = gasPrice + metadata.Data = computedData + metadata.ChainID = service.provider.GetNetworkConfig().NetworkID + metadata.Version = transactionVersion - metadataAsObjectsMap, err := metadata.toObjectsMap() + metadataAsObjectsMap, err := toObjectsMap(metadata) if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrConstructionCheck, err) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) } return &types.ConstructionMetadataResponse{ @@ -203,34 +183,37 @@ func (service *constructionService) ConstructionMetadata( }, nil } +func (service *constructionService) computeData(options *constructionOptions) []byte { + if service.extension.isNativeCurrencySymbol(options.CurrencySymbol) { + return options.Data + } + + // TBD: Add implementation for ESDT. + return make([]byte, 0) +} + // ConstructionPayloads returns an unsigned transaction blob and a collection of payloads that must be signed func (service *constructionService) ConstructionPayloads( _ context.Context, request *types.ConstructionPayloadsRequest, ) (*types.ConstructionPayloadsResponse, *types.Error) { - log.Info("constructionService.ConstructionPayloads()", "metadata", request.Metadata) - - if err := service.checkOperationsAndMeta(request.Operations, request.Metadata); err != nil { - return nil, err - } + log.Debug("constructionService.ConstructionPayloads()", "metadata", request.Metadata) - tx, err := createTransaction(request) + metadata, err := newConstructionMetadata(request.Metadata) if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, err) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) } - txJson, err := json.Marshal(tx) + txJson, err := metadata.toTransactionJson() if err != nil { - return nil, service.errFactory.newErrWithOriginal(ErrMalformedValue, err) + return nil, service.errFactory.newErrWithOriginal(ErrConstruction, err) } - signer := request.Operations[0].Account.Address - return &types.ConstructionPayloadsResponse{ UnsignedTransaction: string(txJson), Payloads: []*types.SigningPayload{ { - AccountIdentifier: addressToAccountIdentifier(signer), + AccountIdentifier: addressToAccountIdentifier(metadata.Sender), SignatureType: types.Ed25519, Bytes: txJson, }, @@ -283,22 +266,6 @@ func (service *constructionService) createOperationsFromPreparedTx(tx *data.Tran return operations } -func createTransaction(request *types.ConstructionPayloadsRequest) (*data.Transaction, error) { - tx := &data.Transaction{} - - requestMetadataBytes, err := json.Marshal(request.Metadata) - if err != nil { - return nil, err - } - - err = json.Unmarshal(requestMetadataBytes, tx) - if err != nil { - return nil, err - } - - return tx, nil -} - func getTxFromRequest(txString string) (*data.Transaction, error) { txBytes := []byte(txString) @@ -382,7 +349,7 @@ func (service *constructionService) ConstructionSubmit( _ context.Context, request *types.ConstructionSubmitRequest, ) (*types.TransactionIdentifierResponse, *types.Error) { - log.Info("constructionService.ConstructionSubmit()", "transaction", request.SignedTransaction) + log.Debug("constructionService.ConstructionSubmit()", "transaction", request.SignedTransaction) if service.provider.IsOffline() { return nil, service.errFactory.newErr(ErrOfflineMode) diff --git a/server/services/constructionServiceFee.go b/server/services/constructionServiceFee.go index db0706db..860cd030 100644 --- a/server/services/constructionServiceFee.go +++ b/server/services/constructionServiceFee.go @@ -3,77 +3,68 @@ package services import ( "math/big" - "github.com/ElrondNetwork/rosetta/server/resources" "github.com/coinbase/rosetta-sdk-go/types" ) -func (service *constructionService) computeSuggestedFeeAndGas(txType string, options *constructionOptions, networkConfig *resources.NetworkConfig) (*big.Int, uint64, uint64, *types.Error) { - gasLimit := options.GasLimit - gasPrice := options.GasPrice +func (service *constructionService) computeSuggestedFeeAndGas(options *constructionOptions, computedData []byte) (*big.Int, uint64, uint64, *types.Error) { + networkConfig := service.provider.GetNetworkConfig() + providedMaxFee := options.getMaxFee() + providedGasLimit := options.GasLimit + providedGasPrice := options.GasPrice + decidedGasLimit := uint64(0) + decidedGasPrice := uint64(0) + // TODO: Handle in a future PR + gasPriceModifier := float64(0.01) + + isForNativeCurrency := service.extension.isNativeCurrencySymbol(options.CurrencySymbol) + isForCustomCurrency := !isForNativeCurrency + if isForCustomCurrency { + // TODO: Handle in a future PR + return nil, 0, 0, service.errFactory.newErr(ErrNotImplemented) + } - if gasLimit > 0 { - err := service.checkProvidedGasLimit(gasLimit, txType, options, networkConfig) - if err != nil { - return nil, 0, 0, err - } - } else { - // if gas limit is not provided, we estimate it - estimatedGasLimit, err := service.estimateGasLimit(txType, networkConfig, options) - if err != nil { - return nil, 0, 0, err - } + movementGasLimit := networkConfig.MinGasLimit + networkConfig.GasPerDataByte*uint64(len(computedData)) + // TODO: Handle in a future PR + executionGasLimit := uint64(0) - gasLimit = estimatedGasLimit + estimatedGasLimit := movementGasLimit + executionGasLimit + if providedGasLimit > 0 { + decidedGasLimit = providedGasLimit + } + if decidedGasLimit < estimatedGasLimit { + return nil, 0, 0, service.errFactory.newErr(ErrInsufficientGasLimit) } - if gasPrice > 0 { - if gasPrice < networkConfig.MinGasPrice { - return nil, 0, 0, service.errFactory.newErr(ErrGasPriceTooLow) - } + if providedGasPrice > 0 { + decidedGasPrice = providedGasPrice } else { - // if gas price is not provided, we set it to minGasPrice - gasPrice = networkConfig.MinGasPrice + decidedGasPrice = networkConfig.MinGasPrice } if options.FeeMultiplier != 0 { - gasPrice = uint64(options.FeeMultiplier * float64(gasPrice)) - - if gasPrice < networkConfig.MinGasPrice { - gasPrice = networkConfig.MinGasPrice - } + decidedGasPrice = uint64(options.FeeMultiplier * float64(decidedGasPrice)) } - - fee := big.NewInt(0).Mul( - big.NewInt(0).SetUint64(gasPrice), - big.NewInt(0).SetUint64(gasLimit), - ) - - // TODO: In the case that the caller provides both a max fee and a suggested fee multiplier, the max fee will set an upper bound on the suggested fee (regardless of the multiplier provided). - - return fee, gasPrice, gasLimit, nil -} - -func (service *constructionService) estimateGasLimit(operationType string, networkConfig *resources.NetworkConfig, options *constructionOptions) (uint64, *types.Error) { - gasForDataField := networkConfig.GasPerDataByte * uint64(len(options.Data)) - - switch operationType { - case opTransfer: - return networkConfig.MinGasLimit + gasForDataField, nil - default: - // we do not support this yet other operation types, but we might support it in the future - return 0, service.errFactory.newErr(ErrNotImplemented) - } -} - -func (service *constructionService) checkProvidedGasLimit(providedGasLimit uint64, txType string, options *constructionOptions, networkConfig *resources.NetworkConfig) *types.Error { - estimatedGasLimit, err := service.estimateGasLimit(txType, networkConfig, options) - if err != nil { - return err + if decidedGasPrice < networkConfig.MinGasPrice { + return nil, 0, 0, service.errFactory.newErr(ErrGasPriceTooLow) } - if providedGasLimit < estimatedGasLimit { - return service.errFactory.newErr(ErrInsufficientGasLimit) + movementFee := multiplyUint64(movementGasLimit, decidedGasPrice) + executionGasPrice := uint64(float64(decidedGasPrice) * gasPriceModifier) + executionFee := multiplyUint64(executionGasLimit, executionGasPrice) + computedFee := addBigInt(movementFee, executionFee) + + // In the case that the caller provides both a max fee and a suggested fee multiplier, + // the max fee will set an upper bound on the suggested fee (regardless of the multiplier provided). + if computedFee.Cmp(providedMaxFee) > 0 { + // We are re-computing the decidedGasPrice, as follows: + // providedMaxFee = movementGasLimit * decidedGasPrice + executionGasLimit * decidedGasPrice * gasPriceModifier + // => + // decidedGasPrice = providedMaxFee / (movementGasLimit + executionGasLimit * gasPriceModifier) + + denominator := big.NewFloat(float64(movementGasLimit) + float64(executionGasLimit)*gasPriceModifier) + recomputedGasPrice := divideBigIntByBigFloat(providedMaxFee, denominator) + decidedGasPrice, _ = recomputedGasPrice.Uint64() } - return nil + return computedFee, decidedGasPrice, decidedGasLimit, nil } diff --git a/server/services/errors.go b/server/services/errors.go index 20d3e139..bdd06a7d 100644 --- a/server/services/errors.go +++ b/server/services/errors.go @@ -18,7 +18,7 @@ const ( ErrMalformedValue ErrUnableToGetNodeStatus ErrMustQueryByIndexOrByHash - ErrConstructionCheck + ErrConstruction ErrUnableToGetNetworkConfig ErrUnsupportedCurveType ErrInsufficientGasLimit @@ -97,8 +97,8 @@ func createErrPrototypes() ([]errPrototype, map[errCode]errPrototype) { retriable: false, }, { - code: ErrConstructionCheck, - message: "operation construction check error", + code: ErrConstruction, + message: "construction error", retriable: false, }, { diff --git a/server/services/networkProviderExtension.go b/server/services/networkProviderExtension.go index df758e5a..0d2341ce 100644 --- a/server/services/networkProviderExtension.go +++ b/server/services/networkProviderExtension.go @@ -40,11 +40,19 @@ func (extension *networkProviderExtension) getNativeCurrency() *types.Currency { } } +func (extension *networkProviderExtension) getNativeCurrencySymbol() string { + return extension.provider.GetNativeCurrency().Symbol +} + func (extension *networkProviderExtension) isNativeCurrency(currency *types.Currency) bool { nativeCurrency := extension.provider.GetNativeCurrency() return currency.Symbol == nativeCurrency.Symbol && currency.Decimals == nativeCurrency.Decimals } +func (extension *networkProviderExtension) isNativeCurrencySymbol(symbol string) bool { + return symbol == extension.getNativeCurrencySymbol() +} + func (extension *networkProviderExtension) getGenesisBlockIdentifier() *types.BlockIdentifier { summary := extension.provider.GetGenesisBlockSummary() return blockSummaryToIdentifier(summary) diff --git a/server/services/operations.go b/server/services/operations.go index 4135dcae..1d0b2dc3 100644 --- a/server/services/operations.go +++ b/server/services/operations.go @@ -28,12 +28,6 @@ var ( opFeeRefund, } - // SupportedOperationTypesForConstruction is a list of the supported operations for transaction construction - SupportedOperationTypesForConstruction = []string{ - opTransfer, - opFee, - } - opStatusSuccess = "Success" opStatusFailure = "Failure" From bcfe87eb575edde441a246e969fbdac515e9f46c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Fri, 7 Oct 2022 17:14:21 +0300 Subject: [PATCH 03/12] Bugfixing (tests in progress). --- server/services/common.go | 2 +- server/services/constructionOptions.go | 4 ++++ server/services/constructionService.go | 6 ++---- server/services/constructionServiceFee.go | 11 +++++++---- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/server/services/common.go b/server/services/common.go index 8a257c96..cadf5fd7 100644 --- a/server/services/common.go +++ b/server/services/common.go @@ -56,5 +56,5 @@ func divideBigIntByBigFloat(numerator *big.Int, denominator *big.Float) *big.Flo } func addBigInt(a *big.Int, b *big.Int) *big.Int { - return big.NewInt(0).And(a, b) + return big.NewInt(0).Add(a, b) } diff --git a/server/services/constructionOptions.go b/server/services/constructionOptions.go index 4529b651..8afd9e2e 100644 --- a/server/services/constructionOptions.go +++ b/server/services/constructionOptions.go @@ -36,6 +36,10 @@ func (options *constructionOptions) getMaxFee() *big.Int { return big.NewInt(0) } +func (options *constructionOptions) hasMaxFee() bool { + return len(options.MaxFee) > 0 +} + func (options *constructionOptions) validate(nativeCurrencySymbol string) error { if len(options.Sender) == 0 { return errors.New("missing option 'sender'") diff --git a/server/services/constructionService.go b/server/services/constructionService.go index 8a05a33e..299e73bc 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -151,13 +151,11 @@ func (service *constructionService) ConstructionMetadata( computedData := service.computeData(requestOptions) - suggestedFee, gasPrice, gasLimit, errTyped := service.computeSuggestedFeeAndGas(requestOptions, computedData) + fee, gasPrice, gasLimit, errTyped := service.computeFeeComponents(requestOptions, computedData) if err != nil { return nil, errTyped } - // - metadata := &constructionMetadata{} metadata.Nonce = account.Account.Nonce metadata.Sender = requestOptions.Sender @@ -178,7 +176,7 @@ func (service *constructionService) ConstructionMetadata( return &types.ConstructionMetadataResponse{ Metadata: metadataAsObjectsMap, SuggestedFee: []*types.Amount{ - service.extension.valueToNativeAmount(suggestedFee.String()), + service.extension.valueToNativeAmount(fee.String()), }, }, nil } diff --git a/server/services/constructionServiceFee.go b/server/services/constructionServiceFee.go index 860cd030..56795c0e 100644 --- a/server/services/constructionServiceFee.go +++ b/server/services/constructionServiceFee.go @@ -6,9 +6,10 @@ import ( "github.com/coinbase/rosetta-sdk-go/types" ) -func (service *constructionService) computeSuggestedFeeAndGas(options *constructionOptions, computedData []byte) (*big.Int, uint64, uint64, *types.Error) { +func (service *constructionService) computeFeeComponents(options *constructionOptions, computedData []byte) (*big.Int, uint64, uint64, *types.Error) { networkConfig := service.provider.GetNetworkConfig() - providedMaxFee := options.getMaxFee() + maxFee := options.getMaxFee() + hasMaxFee := options.hasMaxFee() providedGasLimit := options.GasLimit providedGasPrice := options.GasPrice decidedGasLimit := uint64(0) @@ -55,14 +56,16 @@ func (service *constructionService) computeSuggestedFeeAndGas(options *construct // In the case that the caller provides both a max fee and a suggested fee multiplier, // the max fee will set an upper bound on the suggested fee (regardless of the multiplier provided). - if computedFee.Cmp(providedMaxFee) > 0 { + if hasMaxFee && computedFee.Cmp(maxFee) > 0 { + computedFee = maxFee + // We are re-computing the decidedGasPrice, as follows: // providedMaxFee = movementGasLimit * decidedGasPrice + executionGasLimit * decidedGasPrice * gasPriceModifier // => // decidedGasPrice = providedMaxFee / (movementGasLimit + executionGasLimit * gasPriceModifier) denominator := big.NewFloat(float64(movementGasLimit) + float64(executionGasLimit)*gasPriceModifier) - recomputedGasPrice := divideBigIntByBigFloat(providedMaxFee, denominator) + recomputedGasPrice := divideBigIntByBigFloat(maxFee, denominator) decidedGasPrice, _ = recomputedGasPrice.Uint64() } From 2ff97fecdb9d04f6454b090f2859c51254411a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Fri, 7 Oct 2022 22:21:12 +0300 Subject: [PATCH 04/12] In rosetta specification v1.4.14, maxFee and feeMultiplier do not exist anymore. --- server/services/common.go | 6 - server/services/constructionOptions.go | 34 +-- server/services/constructionService.go | 16 +- server/services/constructionServiceFee.go | 55 ++-- .../services/constructionServiceFee_test.go | 235 ++++++------------ 5 files changed, 111 insertions(+), 235 deletions(-) diff --git a/server/services/common.go b/server/services/common.go index cadf5fd7..b399d5ac 100644 --- a/server/services/common.go +++ b/server/services/common.go @@ -49,12 +49,6 @@ func multiplyUint64(a uint64, b uint64) *big.Int { return big.NewInt(0).Mul(big.NewInt(0).SetUint64(a), big.NewInt(0).SetUint64(b)) } -func divideBigIntByBigFloat(numerator *big.Int, denominator *big.Float) *big.Float { - result := big.NewFloat(0).SetInt(numerator) - result = result.Quo(result, denominator) - return result -} - func addBigInt(a *big.Int, b *big.Int) *big.Int { return big.NewInt(0).Add(a, b) } diff --git a/server/services/constructionOptions.go b/server/services/constructionOptions.go index 8afd9e2e..abfc2ffb 100644 --- a/server/services/constructionOptions.go +++ b/server/services/constructionOptions.go @@ -2,19 +2,16 @@ package services import ( "errors" - "math/big" ) type constructionOptions struct { - Sender string `json:"sender"` - Receiver string `json:"receiver"` - Amount string `json:"amount"` - CurrencySymbol string `json:"currencySymbol"` - GasLimit uint64 `json:"gasLimit"` - GasPrice uint64 `json:"gasPrice"` - Data []byte `json:"data"` - MaxFee string `json:"maxFee"` - FeeMultiplier float64 `json:"feeMultiplier"` + Sender string `json:"sender"` + Receiver string `json:"receiver"` + Amount string `json:"amount"` + CurrencySymbol string `json:"currencySymbol"` + GasLimit uint64 `json:"gasLimit"` + GasPrice uint64 `json:"gasPrice"` + Data []byte `json:"data"` } func newConstructionOptions(obj objectsMap) (*constructionOptions, error) { @@ -27,17 +24,20 @@ func newConstructionOptions(obj objectsMap) (*constructionOptions, error) { return result, nil } -func (options *constructionOptions) getMaxFee() *big.Int { - maxFee, ok := big.NewInt(0).SetString(options.MaxFee, 10) - if ok { - return maxFee +func (options *constructionOptions) coalesceGasLimit(estimatedGasLimit uint64) uint64 { + if options.GasLimit == 0 { + return estimatedGasLimit } - return big.NewInt(0) + return options.GasLimit } -func (options *constructionOptions) hasMaxFee() bool { - return len(options.MaxFee) > 0 +func (options *constructionOptions) coalesceGasPrice(minGasPrice uint64) uint64 { + if options.GasPrice == 0 { + return minGasPrice + } + + return options.GasPrice } func (options *constructionOptions) validate(nativeCurrencySymbol string) error { diff --git a/server/services/constructionService.go b/server/services/constructionService.go index 299e73bc..f91878f8 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -35,8 +35,6 @@ func (service *constructionService) ConstructionPreprocess( ) (*types.ConstructionPreprocessResponse, *types.Error) { log.Debug("constructionService.ConstructionPreprocess()", "metadata", request.Metadata, - "maxFee", request.MaxFee, - "suggestedFeeMultiplier", request.SuggestedFeeMultiplier, ) noOperationProvided := len(request.Operations) == 0 @@ -89,18 +87,6 @@ func (service *constructionService) ConstructionPreprocess( responseOptions.CurrencySymbol = request.Operations[0].Amount.Currency.Symbol } - if len(request.MaxFee) > 0 { - maxFee := request.MaxFee[0] - if !service.extension.isNativeCurrency(maxFee.Currency) { - return nil, service.errFactory.newErrWithOriginal(ErrConstruction, errors.New("invalid currency for fee")) - } - - responseOptions.MaxFee = maxFee.Value - } - - if request.SuggestedFeeMultiplier != nil { - responseOptions.FeeMultiplier = *request.SuggestedFeeMultiplier - } if requestMetadata.GasLimit > 0 { responseOptions.GasLimit = requestMetadata.GasLimit } @@ -151,7 +137,7 @@ func (service *constructionService) ConstructionMetadata( computedData := service.computeData(requestOptions) - fee, gasPrice, gasLimit, errTyped := service.computeFeeComponents(requestOptions, computedData) + fee, gasLimit, gasPrice, errTyped := service.computeFeeComponents(requestOptions, computedData) if err != nil { return nil, errTyped } diff --git a/server/services/constructionServiceFee.go b/server/services/constructionServiceFee.go index 56795c0e..6ad8b130 100644 --- a/server/services/constructionServiceFee.go +++ b/server/services/constructionServiceFee.go @@ -8,12 +8,7 @@ import ( func (service *constructionService) computeFeeComponents(options *constructionOptions, computedData []byte) (*big.Int, uint64, uint64, *types.Error) { networkConfig := service.provider.GetNetworkConfig() - maxFee := options.getMaxFee() - hasMaxFee := options.hasMaxFee() - providedGasLimit := options.GasLimit - providedGasPrice := options.GasPrice - decidedGasLimit := uint64(0) - decidedGasPrice := uint64(0) + minGasPrice := networkConfig.MinGasPrice // TODO: Handle in a future PR gasPriceModifier := float64(0.01) @@ -27,47 +22,27 @@ func (service *constructionService) computeFeeComponents(options *constructionOp movementGasLimit := networkConfig.MinGasLimit + networkConfig.GasPerDataByte*uint64(len(computedData)) // TODO: Handle in a future PR executionGasLimit := uint64(0) - estimatedGasLimit := movementGasLimit + executionGasLimit - if providedGasLimit > 0 { - decidedGasLimit = providedGasLimit - } - if decidedGasLimit < estimatedGasLimit { - return nil, 0, 0, service.errFactory.newErr(ErrInsufficientGasLimit) - } - if providedGasPrice > 0 { - decidedGasPrice = providedGasPrice - } else { - decidedGasPrice = networkConfig.MinGasPrice - } + gasLimit := options.coalesceGasLimit(estimatedGasLimit) + gasPrice := options.coalesceGasPrice(minGasPrice) - if options.FeeMultiplier != 0 { - decidedGasPrice = uint64(options.FeeMultiplier * float64(decidedGasPrice)) + if gasLimit < estimatedGasLimit { + return nil, 0, 0, service.errFactory.newErr(ErrInsufficientGasLimit) } - if decidedGasPrice < networkConfig.MinGasPrice { + if gasPrice < minGasPrice { return nil, 0, 0, service.errFactory.newErr(ErrGasPriceTooLow) } - movementFee := multiplyUint64(movementGasLimit, decidedGasPrice) - executionGasPrice := uint64(float64(decidedGasPrice) * gasPriceModifier) - executionFee := multiplyUint64(executionGasLimit, executionGasPrice) - computedFee := addBigInt(movementFee, executionFee) - - // In the case that the caller provides both a max fee and a suggested fee multiplier, - // the max fee will set an upper bound on the suggested fee (regardless of the multiplier provided). - if hasMaxFee && computedFee.Cmp(maxFee) > 0 { - computedFee = maxFee + fee := computeFee(movementGasLimit, executionGasLimit, gasPrice, gasPriceModifier) - // We are re-computing the decidedGasPrice, as follows: - // providedMaxFee = movementGasLimit * decidedGasPrice + executionGasLimit * decidedGasPrice * gasPriceModifier - // => - // decidedGasPrice = providedMaxFee / (movementGasLimit + executionGasLimit * gasPriceModifier) - - denominator := big.NewFloat(float64(movementGasLimit) + float64(executionGasLimit)*gasPriceModifier) - recomputedGasPrice := divideBigIntByBigFloat(maxFee, denominator) - decidedGasPrice, _ = recomputedGasPrice.Uint64() - } + return fee, gasLimit, gasPrice, nil +} - return computedFee, decidedGasPrice, decidedGasLimit, nil +func computeFee(movementGasLimit uint64, executionGasLimit uint64, gasPrice uint64, gasPriceModifier float64) *big.Int { + movementFee := multiplyUint64(movementGasLimit, gasPrice) + executionGasPrice := uint64(float64(gasPrice) * gasPriceModifier) + executionFee := multiplyUint64(executionGasLimit, executionGasPrice) + computedFee := addBigInt(movementFee, executionFee) + return computedFee } diff --git a/server/services/constructionServiceFee_test.go b/server/services/constructionServiceFee_test.go index 5c002ac2..68a99ddb 100644 --- a/server/services/constructionServiceFee_test.go +++ b/server/services/constructionServiceFee_test.go @@ -1,172 +1,93 @@ package services import ( - "math/big" "testing" - "github.com/ElrondNetwork/rosetta/server/resources" "github.com/ElrondNetwork/rosetta/testscommon" "github.com/stretchr/testify/require" ) -func TestEstimateGasLimit(t *testing.T) { +func TestConstructionService_ComputeFeeComponents(t *testing.T) { t.Parallel() networkProvider := testscommon.NewNetworkProviderMock() service := NewConstructionService(networkProvider).(*constructionService) - minGasLimit := uint64(1000) - gasPerDataByte := uint64(100) - networkConfig := &resources.NetworkConfig{ - GasPerDataByte: gasPerDataByte, - MinGasLimit: minGasLimit, - } - - dataField := "transaction-data" - options := objectsMap{ - "data": dataField, - } - - expectedGasLimit := minGasLimit + uint64(len(dataField))*gasPerDataByte - - gasLimit, err := service.estimateGasLimit(opTransfer, networkConfig, options) - require.Nil(t, err) - require.Equal(t, expectedGasLimit, gasLimit) - - gasLimit, err = service.estimateGasLimit(opTransfer, networkConfig, nil) - require.Nil(t, err) - require.Equal(t, minGasLimit, gasLimit) - - // Unsupported operation type (you cannot estimate gasLimit for e.g. a reward operation) - gasLimit, err = service.estimateGasLimit(opReward, networkConfig, nil) - require.Equal(t, ErrNotImplemented, errCode(err.Code)) - require.Equal(t, uint64(0), gasLimit) -} - -func TestProvidedGasLimit(t *testing.T) { - t.Parallel() - - networkProvider := testscommon.NewNetworkProviderMock() - service := NewConstructionService(networkProvider).(*constructionService) - - minGasLimit := uint64(1000) - gasPerDataByte := uint64(100) - networkConfig := &resources.NetworkConfig{ - GasPerDataByte: gasPerDataByte, - MinGasLimit: minGasLimit, - } - - dataField := "transaction-data" - options := objectsMap{ - "data": dataField, - } - - err := service.checkProvidedGasLimit(uint64(900), opTransfer, options, networkConfig) - require.Equal(t, ErrInsufficientGasLimit, errCode(err.Code)) - - err = service.checkProvidedGasLimit(uint64(900), opReward, options, networkConfig) - require.Equal(t, ErrNotImplemented, errCode(err.Code)) - - err = service.checkProvidedGasLimit(uint64(9000), opTransfer, options, networkConfig) - require.Nil(t, err) -} - -func TestAdjustTxFeeWithFeeMultiplier(t *testing.T) { - t.Parallel() - - networkProvider := testscommon.NewNetworkProviderMock() - service := NewConstructionService(networkProvider).(*constructionService) - - options := objectsMap{ - "feeMultiplier": 1.1, - } - - expectedGasPrice := uint64(1100) - expectedFee := "1100" - suggestedFee := big.NewInt(1000) - - suggestedFeeResult, gasPriceResult := service.adjustTxFeeWithFeeMultiplier(suggestedFee, 1000, options, 1000) - require.Equal(t, expectedFee, suggestedFeeResult.String()) - require.Equal(t, expectedGasPrice, gasPriceResult) - - expectedGasPrice = uint64(1000) - expectedFee = "1000" - suggestedFeeResult, gasPriceResult = service.adjustTxFeeWithFeeMultiplier(suggestedFee, 1000, make(objectsMap), 1000) - require.Equal(t, expectedFee, suggestedFeeResult.String()) - require.Equal(t, expectedGasPrice, gasPriceResult) -} - -func TestComputeSuggestedFeeAndGas(t *testing.T) { - t.Parallel() - - networkProvider := testscommon.NewNetworkProviderMock() - service := NewConstructionService(networkProvider).(*constructionService) - - minGasLimit := uint64(1000) - minGasPrice := uint64(10) - gasPerDataByte := uint64(100) - networkConfig := &resources.NetworkConfig{ - GasPerDataByte: gasPerDataByte, - MinGasLimit: minGasLimit, - MinGasPrice: minGasPrice, - } - - providedGasPrice := uint64(10) - options := objectsMap{ - "gasPrice": providedGasPrice, - } - - suggestedFee, gasPrice, gasLimit, err := service.computeSuggestedFeeAndGas(opTransfer, options, networkConfig) - require.Nil(t, err) - require.Equal(t, minGasLimit, gasLimit) - require.Equal(t, big.NewInt(10000), suggestedFee) - require.Equal(t, providedGasPrice, gasPrice) - - // err provided gas price is too low - options["gasPrice"] = 1 - _, _, _, err = service.computeSuggestedFeeAndGas(opTransfer, options, networkConfig) - require.Equal(t, ErrGasPriceTooLow, errCode(err.Code)) - - // err provided gas limit is too low - options["gasPrice"] = minGasPrice - options["gasLimit"] = 1 - _, _, _, err = service.computeSuggestedFeeAndGas(opTransfer, options, networkConfig) - require.Equal(t, ErrInsufficientGasLimit, errCode(err.Code)) - - delete(options, "gasLimit") - options["gasPrice"] = minGasPrice - _, _, _, err = service.computeSuggestedFeeAndGas(opReward, options, networkConfig) - require.Equal(t, ErrNotImplemented, errCode(err.Code)) - - //check with fee multiplier - delete(options, "gasPrice") - delete(options, "gasLimit") - options["feeMultiplier"] = 1.1 - expectedSuggestedFee := big.NewInt(11000) - expectedGasPrice := uint64(11) - suggestedFee, gasPrice, gasLimit, err = service.computeSuggestedFeeAndGas(opTransfer, options, networkConfig) - require.Nil(t, err) - require.Equal(t, minGasLimit, gasLimit) - require.Equal(t, expectedSuggestedFee, suggestedFee) - require.Equal(t, expectedGasPrice, gasPrice) -} - -func TestAdjustTxFeeWithFeeMultiplier_FeeMultiplierLessThanOne(t *testing.T) { - t.Parallel() - - networkProvider := testscommon.NewNetworkProviderMock() - service := NewConstructionService(networkProvider).(*constructionService) - - options := objectsMap{ - "feeMultiplier": 0.5, - } - - expectedFee := "500" - suggestedFee := big.NewInt(1000) - gasPrice := uint64(1000) - minGasPrice := uint64(900) - - suggestedFeeResult, gasPriceResult := service.adjustTxFeeWithFeeMultiplier(suggestedFee, gasPrice, options, minGasPrice) - require.Equal(t, expectedFee, suggestedFeeResult.String()) - require.Equal(t, minGasPrice, gasPriceResult) + t.Run("native transfer", func(t *testing.T) { + fee, gasLimit, gasPrice, err := service.computeFeeComponents(&constructionOptions{ + GasLimit: 50000, + GasPrice: 1000000000, + CurrencySymbol: "XeGLD", + }, []byte{}) + + require.Nil(t, err) + require.Equal(t, "50000000000000", fee.String()) + require.Equal(t, uint64(50000), gasLimit) + require.Equal(t, uint64(1000000000), gasPrice) + }) + + t.Run("native transfer, with computed data", func(t *testing.T) { + fee, gasLimit, gasPrice, err := service.computeFeeComponents(&constructionOptions{ + GasLimit: 53000, + GasPrice: 1000000000, + CurrencySymbol: "XeGLD", + }, []byte{0xaa, 0xbb}) + + require.Nil(t, err) + require.Equal(t, "53000000000000", fee.String()) + require.Equal(t, uint64(53000), gasLimit) + require.Equal(t, uint64(1000000000), gasPrice) + }) + + t.Run("native transfer, with insufficient gas limit", func(t *testing.T) { + fee, gasLimit, gasPrice, err := service.computeFeeComponents(&constructionOptions{ + GasLimit: 40000, + GasPrice: 1000000000, + CurrencySymbol: "XeGLD", + }, []byte{}) + + require.Equal(t, int32(ErrInsufficientGasLimit), err.Code) + require.Nil(t, fee) + require.Equal(t, uint64(0), gasLimit) + require.Equal(t, uint64(0), gasPrice) + }) + + t.Run("native transfer, with more gas limit than necessary", func(t *testing.T) { + fee, gasLimit, gasPrice, err := service.computeFeeComponents(&constructionOptions{ + GasLimit: 70000, + GasPrice: 1000000000, + CurrencySymbol: "XeGLD", + }, []byte{}) + + require.Nil(t, err) + require.Equal(t, "50000000000000", fee.String()) + require.Equal(t, uint64(70000), gasLimit) + require.Equal(t, uint64(1000000000), gasPrice) + }) + + t.Run("native transfer, with gas price too low", func(t *testing.T) { + fee, gasLimit, gasPrice, err := service.computeFeeComponents(&constructionOptions{ + GasLimit: 50000, + GasPrice: 500000000, + CurrencySymbol: "XeGLD", + }, []byte{}) + + require.Equal(t, int32(ErrGasPriceTooLow), err.Code) + require.Nil(t, fee) + require.Equal(t, uint64(0), gasLimit) + require.Equal(t, uint64(0), gasPrice) + }) + + t.Run("native transfer, with gas price higher than necessary", func(t *testing.T) { + fee, gasLimit, gasPrice, err := service.computeFeeComponents(&constructionOptions{ + GasLimit: 50000, + GasPrice: 2000000000, + CurrencySymbol: "XeGLD", + }, []byte{}) + + require.Nil(t, err) + require.Equal(t, "100000000000000", fee.String()) + require.Equal(t, uint64(50000), gasLimit) + require.Equal(t, uint64(2000000000), gasPrice) + }) } From 8129bf5d28cc7582550f6154f5e442e65793ce35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Sun, 9 Oct 2022 20:37:41 +0300 Subject: [PATCH 05/12] Add unit test. --- server/services/constructionServiceFee_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/server/services/constructionServiceFee_test.go b/server/services/constructionServiceFee_test.go index 68a99ddb..6281f76e 100644 --- a/server/services/constructionServiceFee_test.go +++ b/server/services/constructionServiceFee_test.go @@ -1,6 +1,7 @@ package services import ( + "math/big" "testing" "github.com/ElrondNetwork/rosetta/testscommon" @@ -91,3 +92,13 @@ func TestConstructionService_ComputeFeeComponents(t *testing.T) { require.Equal(t, uint64(2000000000), gasPrice) }) } + +func TestComputeFee(t *testing.T) { + t.Parallel() + + require.Equal(t, big.NewInt(50000000000000), computeFee(50000, 0, 1000000000, 0.01)) + require.Equal(t, big.NewInt(50000000000000), computeFee(50000, 0, 1000000000, 0.02)) + require.Equal(t, big.NewInt(100000000000000), computeFee(50000, 0, 2000000000, 0.01)) + require.Equal(t, big.NewInt(70000000000000), computeFee(70000, 0, 1000000000, 0.01)) + require.Equal(t, big.NewInt(60000000000000), computeFee(50000, 1000000, 1000000000, 0.01)) +} From ef3e1e9b3d2df4d73209659a4b78e01a22fee4a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Sun, 9 Oct 2022 23:31:03 +0300 Subject: [PATCH 06/12] Add / adjust unit tests (some of the existing tests do not compile yet). --- server/services/constructionOptions.go | 2 +- server/services/constructionService.go | 4 +- server/services/constructionServiceFee.go | 1 - server/services/constructionService_test.go | 217 +++++++++++++------- 4 files changed, 149 insertions(+), 75 deletions(-) diff --git a/server/services/constructionOptions.go b/server/services/constructionOptions.go index abfc2ffb..9c2fee5d 100644 --- a/server/services/constructionOptions.go +++ b/server/services/constructionOptions.go @@ -54,7 +54,7 @@ func (options *constructionOptions) validate(nativeCurrencySymbol string) error return errors.New("missing option 'currencySymbol'") } if len(options.Data) > 0 && options.CurrencySymbol != nativeCurrencySymbol { - return errors.New("for custom currencies, cannot populate option 'data'") + return errors.New("for custom currencies, option 'data' must be empty") } return nil diff --git a/server/services/constructionService.go b/server/services/constructionService.go index f91878f8..0b509837 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -33,9 +33,7 @@ func (service *constructionService) ConstructionPreprocess( _ context.Context, request *types.ConstructionPreprocessRequest, ) (*types.ConstructionPreprocessResponse, *types.Error) { - log.Debug("constructionService.ConstructionPreprocess()", - "metadata", request.Metadata, - ) + log.Debug("constructionService.ConstructionPreprocess()", "metadata", request.Metadata) noOperationProvided := len(request.Operations) == 0 lessThanTwoOperationsProvided := len(request.Operations) < 2 diff --git a/server/services/constructionServiceFee.go b/server/services/constructionServiceFee.go index 6ad8b130..3c794fee 100644 --- a/server/services/constructionServiceFee.go +++ b/server/services/constructionServiceFee.go @@ -35,7 +35,6 @@ func (service *constructionService) computeFeeComponents(options *constructionOp } fee := computeFee(movementGasLimit, executionGasLimit, gasPrice, gasPriceModifier) - return fee, gasLimit, gasPrice, nil } diff --git a/server/services/constructionService_test.go b/server/services/constructionService_test.go index efdb9fdf..8b369a2a 100644 --- a/server/services/constructionService_test.go +++ b/server/services/constructionService_test.go @@ -12,58 +12,98 @@ import ( ) func TestConstructionService_ConstructionPreprocess(t *testing.T) { + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() extension := newNetworkProviderExtension(networkProvider) service := NewConstructionService(networkProvider) - operations := []*types.Operation{ - { - OperationIdentifier: indexToOperationIdentifier(0), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressAlice), - Amount: extension.valueToNativeAmount("-1234"), - }, - { - OperationIdentifier: indexToOperationIdentifier(1), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressBob), - Amount: extension.valueToNativeAmount("1234"), - }, - } + t.Run("with minimal (empty) 'metadata', 'options' being inferred from 'operations'", func(t *testing.T) { + operations := []*types.Operation{ + { + OperationIdentifier: indexToOperationIdentifier(0), + Type: opTransfer, + Account: addressToAccountIdentifier(testscommon.TestAddressAlice), + Amount: extension.valueToNativeAmount("-1234"), + }, + { + OperationIdentifier: indexToOperationIdentifier(1), + Type: opTransfer, + Account: addressToAccountIdentifier(testscommon.TestAddressBob), + Amount: extension.valueToNativeAmount("1234"), + }, + } - feeMultiplier := 1.1 + response, err := service.ConstructionPreprocess(context.Background(), + &types.ConstructionPreprocessRequest{ + Operations: operations, + Metadata: objectsMap{}, + }, + ) - response, err := service.ConstructionPreprocess(context.Background(), - &types.ConstructionPreprocessRequest{ - Operations: operations, - MaxFee: []*types.Amount{ - extension.valueToNativeAmount("123"), + expectedOptions := &constructionOptions{ + Sender: testscommon.TestAddressAlice, + Receiver: testscommon.TestAddressBob, + Amount: "1234", + CurrencySymbol: "XeGLD", + } + + actualOptions := &constructionOptions{} + _ = fromObjectsMap(response.Options, actualOptions) + + require.Nil(t, err) + require.Equal(t, expectedOptions, actualOptions) + }) + + t.Run("with maximal 'metadata', without 'operations'", func(t *testing.T) { + response, err := service.ConstructionPreprocess(context.Background(), + &types.ConstructionPreprocessRequest{ + Metadata: objectsMap{ + "sender": testscommon.TestAddressAlice, + "receiver": testscommon.TestAddressBob, + "amount": "1234", + "currencySymbol": "XeGLD", + "gasLimit": 70000, + "gasPrice": 1000000000, + "data": []byte("hello"), + }, }, - SuggestedFeeMultiplier: &feeMultiplier, - Metadata: objectsMap{ - "gasPrice": 1000000000, - "gasLimit": 50000, - "data": "hello", + ) + + expectedOptions := &constructionOptions{ + Sender: testscommon.TestAddressAlice, + Receiver: testscommon.TestAddressBob, + Amount: "1234", + CurrencySymbol: "XeGLD", + GasLimit: 70000, + GasPrice: 1000000000, + Data: []byte("hello"), + } + + actualOptions := &constructionOptions{} + _ = fromObjectsMap(response.Options, actualOptions) + + require.Nil(t, err) + require.Equal(t, expectedOptions, actualOptions) + }) + + t.Run("with incomplete 'metadata', without 'operations'", func(t *testing.T) { + response, err := service.ConstructionPreprocess(context.Background(), + &types.ConstructionPreprocessRequest{ + Metadata: objectsMap{ + "sender": testscommon.TestAddressAlice, + "receiver": testscommon.TestAddressBob, + }, }, - }, - ) - require.Nil(t, err) - require.Equal(t, map[string]interface{}{ - "receiver": testscommon.TestAddressBob, - "sender": testscommon.TestAddressAlice, - "gasPrice": 1000000000, - "gasLimit": 50000, - "feeMultiplier": feeMultiplier, - "data": "hello", - "value": "1234", - "maxFee": "123", - "type": opTransfer, - }, response.Options) + ) + + require.Equal(t, int32(ErrConstruction), err.Code) + require.Nil(t, response) + }) } func TestConstructionService_ConstructionMetadata(t *testing.T) { networkProvider := testscommon.NewNetworkProviderMock() - networkProvider.MockNetworkConfig.NetworkID = "T" networkProvider.MockAccountsByAddress[testscommon.TestAddressAlice] = &resources.Account{ Address: testscommon.TestAddressAlice, Nonce: 42, @@ -71,39 +111,76 @@ func TestConstructionService_ConstructionMetadata(t *testing.T) { service := NewConstructionService(networkProvider) - options := map[string]interface{}{ - "receiver": testscommon.TestAddressBob, - "sender": testscommon.TestAddressAlice, - "gasPrice": uint64(1000000000), - "gasLimit": uint64(57500), - "feeMultiplier": 1.1, - "data": "hello", - "value": "1234", - "maxFee": "1", - "type": opTransfer, - } - response, err := service.ConstructionMetadata(context.Background(), - &types.ConstructionMetadataRequest{ - Options: options, - }, - ) + t.Run("with explicitly providing gas limit and price", func(t *testing.T) { + response, err := service.ConstructionMetadata(context.Background(), + &types.ConstructionMetadataRequest{ + Options: objectsMap{ + "receiver": testscommon.TestAddressBob, + "sender": testscommon.TestAddressAlice, + "amount": "1234", + "currencySymbol": "XeGLD", + "gasLimit": 70000, + "gasPrice": 1000000000, + "data": []byte("hello"), + }, + }, + ) + + expectedMetadata := &constructionMetadata{ + Sender: testscommon.TestAddressAlice, + Receiver: testscommon.TestAddressBob, + Nonce: 42, + Amount: "1234", + CurrencySymbol: "XeGLD", + GasLimit: 70000, + GasPrice: 1000000000, + Data: []byte("hello"), + ChainID: "T", + Version: 1, + } - require.Nil(t, err) - require.Equal(t, "63250000000000", response.SuggestedFee[0].Value) + actualMetadata := &constructionMetadata{} + _ = fromObjectsMap(response.Metadata, actualMetadata) - expectedMetadata := map[string]interface{}{ - "receiver": testscommon.TestAddressBob, - "sender": testscommon.TestAddressAlice, - "chainID": "T", - "version": 1, - "data": []byte("hello"), - "value": "1234", - "nonce": uint64(42), - "gasPrice": uint64(1100000000), - "gasLimit": uint64(57500), - } + require.Nil(t, err) + // We are suggesting the fee by considering the refund + require.Equal(t, "57500000000000", response.SuggestedFee[0].Value) + require.Equal(t, expectedMetadata, actualMetadata) + }) - require.Equal(t, expectedMetadata, response.Metadata) + t.Run("without providing gas limit and price", func(t *testing.T) { + response, err := service.ConstructionMetadata(context.Background(), + &types.ConstructionMetadataRequest{ + Options: objectsMap{ + "receiver": testscommon.TestAddressBob, + "sender": testscommon.TestAddressAlice, + "amount": "1234", + "currencySymbol": "XeGLD", + "data": []byte("hello"), + }, + }, + ) + + expectedMetadata := &constructionMetadata{ + Sender: testscommon.TestAddressAlice, + Receiver: testscommon.TestAddressBob, + Nonce: 42, + Amount: "1234", + CurrencySymbol: "XeGLD", + GasLimit: 57500, + GasPrice: 1000000000, + Data: []byte("hello"), + ChainID: "T", + Version: 1, + } + + actualMetadata := &constructionMetadata{} + _ = fromObjectsMap(response.Metadata, actualMetadata) + + require.Nil(t, err) + require.Equal(t, "57500000000000", response.SuggestedFee[0].Value) + require.Equal(t, expectedMetadata, actualMetadata) + }) } func TestConstructionService_ConstructionPayloads(t *testing.T) { From 2a2d25c25017accb301535c4b8242f45357a8c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Mon, 10 Oct 2022 12:47:55 +0300 Subject: [PATCH 07/12] Update / fix tests. --- server/services/constructionMetadata.go | 2 +- server/services/constructionService_test.go | 280 ++++++++++---------- 2 files changed, 137 insertions(+), 145 deletions(-) diff --git a/server/services/constructionMetadata.go b/server/services/constructionMetadata.go index 5281a472..b54e64b8 100644 --- a/server/services/constructionMetadata.go +++ b/server/services/constructionMetadata.go @@ -57,7 +57,7 @@ func (metadata *constructionMetadata) toTransaction() (*data.Transaction, error) tx.Value = metadata.Amount tx.GasLimit = metadata.GasLimit tx.GasPrice = metadata.GasPrice - tx.Data = nil + tx.Data = metadata.Data tx.ChainID = metadata.ChainID tx.Version = uint32(metadata.Version) diff --git a/server/services/constructionService_test.go b/server/services/constructionService_test.go index 8b369a2a..16aec948 100644 --- a/server/services/constructionService_test.go +++ b/server/services/constructionService_test.go @@ -19,6 +19,8 @@ func TestConstructionService_ConstructionPreprocess(t *testing.T) { service := NewConstructionService(networkProvider) t.Run("with minimal (empty) 'metadata', 'options' being inferred from 'operations'", func(t *testing.T) { + t.Parallel() + operations := []*types.Operation{ { OperationIdentifier: indexToOperationIdentifier(0), @@ -55,7 +57,103 @@ func TestConstructionService_ConstructionPreprocess(t *testing.T) { require.Equal(t, expectedOptions, actualOptions) }) + t.Run("with one operation, with metadata having: 'receiver'", func(t *testing.T) { + t.Parallel() + + operations := []*types.Operation{ + { + OperationIdentifier: indexToOperationIdentifier(0), + Type: opTransfer, + Account: addressToAccountIdentifier(testscommon.TestAddressAlice), + Amount: extension.valueToNativeAmount("-1234"), + }, + } + + response, err := service.ConstructionPreprocess(context.Background(), + &types.ConstructionPreprocessRequest{ + Operations: operations, + Metadata: objectsMap{ + "receiver": testscommon.TestAddressBob, + }, + }, + ) + + expectedOptions := &constructionOptions{ + Sender: testscommon.TestAddressAlice, + Receiver: testscommon.TestAddressBob, + Amount: "1234", + CurrencySymbol: "XeGLD", + } + + actualOptions := &constructionOptions{} + _ = fromObjectsMap(response.Options, actualOptions) + + require.Nil(t, err) + require.Equal(t, expectedOptions, actualOptions) + }) + + t.Run("with one operation, and metadata having: 'receiver', 'amount'", func(t *testing.T) { + t.Parallel() + + operations := []*types.Operation{ + { + OperationIdentifier: indexToOperationIdentifier(0), + Type: opTransfer, + Account: addressToAccountIdentifier(testscommon.TestAddressAlice), + Amount: extension.valueToNativeAmount("ignored"), + }, + } + + response, err := service.ConstructionPreprocess(context.Background(), + &types.ConstructionPreprocessRequest{ + Operations: operations, + Metadata: objectsMap{ + "receiver": testscommon.TestAddressBob, + "amount": "1234", + }, + }, + ) + + expectedOptions := &constructionOptions{ + Sender: testscommon.TestAddressAlice, + Receiver: testscommon.TestAddressBob, + Amount: "1234", + CurrencySymbol: "XeGLD", + } + + actualOptions := &constructionOptions{} + _ = fromObjectsMap(response.Options, actualOptions) + + require.Nil(t, err) + require.Equal(t, expectedOptions, actualOptions) + }) + + t.Run("with one operation, and missing metadata: 'receiver'", func(t *testing.T) { + t.Parallel() + + operations := []*types.Operation{ + { + OperationIdentifier: indexToOperationIdentifier(0), + Type: opTransfer, + Account: addressToAccountIdentifier(testscommon.TestAddressAlice), + Amount: extension.valueToNativeAmount("1234"), + }, + } + + response, err := service.ConstructionPreprocess(context.Background(), + &types.ConstructionPreprocessRequest{ + Operations: operations, + Metadata: objectsMap{}, + }, + ) + + require.Equal(t, int32(ErrConstruction), err.Code) + require.Nil(t, response) + }) + t.Run("with maximal 'metadata', without 'operations'", func(t *testing.T) { + t.Parallel() + response, err := service.ConstructionPreprocess(context.Background(), &types.ConstructionPreprocessRequest{ Metadata: objectsMap{ @@ -88,6 +186,8 @@ func TestConstructionService_ConstructionPreprocess(t *testing.T) { }) t.Run("with incomplete 'metadata', without 'operations'", func(t *testing.T) { + t.Parallel() + response, err := service.ConstructionPreprocess(context.Background(), &types.ConstructionPreprocessRequest{ Metadata: objectsMap{ @@ -112,6 +212,8 @@ func TestConstructionService_ConstructionMetadata(t *testing.T) { service := NewConstructionService(networkProvider) t.Run("with explicitly providing gas limit and price", func(t *testing.T) { + t.Parallel() + response, err := service.ConstructionMetadata(context.Background(), &types.ConstructionMetadataRequest{ Options: objectsMap{ @@ -149,6 +251,8 @@ func TestConstructionService_ConstructionMetadata(t *testing.T) { }) t.Run("without providing gas limit and price", func(t *testing.T) { + t.Parallel() + response, err := service.ConstructionMetadata(context.Background(), &types.ConstructionMetadataRequest{ Options: objectsMap{ @@ -184,65 +288,47 @@ func TestConstructionService_ConstructionMetadata(t *testing.T) { } func TestConstructionService_ConstructionPayloads(t *testing.T) { + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() - networkProvider.MockNetworkConfig.NetworkID = "T" networkProvider.MockAccountsByAddress[testscommon.TestAddressAlice] = &resources.Account{ Address: testscommon.TestAddressAlice, Nonce: 42, } - extension := newNetworkProviderExtension(networkProvider) service := NewConstructionService(networkProvider) - metadata := map[string]interface{}{ - "receiver": testscommon.TestAddressBob, - "sender": testscommon.TestAddressAlice, - "chainID": "T", - "version": 1, - "data": []byte("hello"), - "value": "1234", - "nonce": uint64(42), - "gasPrice": uint64(1100000000), - "gasLimit": uint64(57500), - } - - operations := []*types.Operation{ - { - OperationIdentifier: indexToOperationIdentifier(0), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressAlice), - Amount: extension.valueToNativeAmount("-1234"), - }, - { - OperationIdentifier: indexToOperationIdentifier(1), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressBob), - Amount: extension.valueToNativeAmount("1234"), - }, - } - response, err := service.ConstructionPayloads(context.Background(), &types.ConstructionPayloadsRequest{ - Operations: operations, - Metadata: metadata, + Metadata: objectsMap{ + "sender": testscommon.TestAddressAlice, + "receiver": testscommon.TestAddressBob, + "nonce": 42, + "amount": "1234", + "currencySymbol": "XeGLD", + "gasLimit": 57500, + "gasPrice": 1000000000, + "data": []byte("hello"), + "chainID": "T", + "version": 1, + }, }, ) - unsignedTx := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1100000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` - unsignedTxBytes := []byte(unsignedTx) + expectedTxJson := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1000000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` require.Nil(t, err) - firstPayload := response.Payloads[0] - require.Equal(t, unsignedTx, response.UnsignedTransaction) - require.Equal(t, unsignedTxBytes, firstPayload.Bytes) - require.Equal(t, testscommon.TestAddressAlice, firstPayload.AccountIdentifier.Address) - require.Equal(t, types.Ed25519, firstPayload.SignatureType) + require.Len(t, response.Payloads, 1) + require.Equal(t, expectedTxJson, response.UnsignedTransaction) + require.Equal(t, []byte(expectedTxJson), response.Payloads[0].Bytes) + require.Equal(t, testscommon.TestAddressAlice, response.Payloads[0].AccountIdentifier.Address) + require.Equal(t, types.Ed25519, response.Payloads[0].SignatureType) } func TestConstructionService_ConstructionParse(t *testing.T) { - networkProvider := testscommon.NewNetworkProviderMock() - networkProvider.MockNetworkConfig.NetworkID = "T" + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() extension := newNetworkProviderExtension(networkProvider) service := NewConstructionService(networkProvider) @@ -275,9 +361,9 @@ func TestConstructionService_ConstructionParse(t *testing.T) { } func TestConstructionService_ConstructionCombine(t *testing.T) { - networkProvider := testscommon.NewNetworkProviderMock() - networkProvider.MockNetworkConfig.NetworkID = "T" + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() service := NewConstructionService(networkProvider) unsignedTx := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1100000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` @@ -299,6 +385,8 @@ func TestConstructionService_ConstructionCombine(t *testing.T) { } func TestConstructionService_ConstructionDerive(t *testing.T) { + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() service := NewConstructionService(networkProvider) @@ -316,6 +404,8 @@ func TestConstructionService_ConstructionDerive(t *testing.T) { } func TestConstructionService_ConstructionHash(t *testing.T) { + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() networkProvider.MockComputedTransactionHash = "aaaa" service := NewConstructionService(networkProvider) @@ -332,6 +422,8 @@ func TestConstructionService_ConstructionHash(t *testing.T) { } func TestConstructionService_ConstructionSubmit(t *testing.T) { + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() var calledWithTransaction *data.Transaction @@ -356,6 +448,8 @@ func TestConstructionService_ConstructionSubmit(t *testing.T) { } func TestConstructionService_CreateOperationsFromPreparedTx(t *testing.T) { + t.Parallel() + networkProvider := testscommon.NewNetworkProviderMock() extension := newNetworkProviderExtension(networkProvider) service := NewConstructionService(networkProvider).(*constructionService) @@ -384,105 +478,3 @@ func TestConstructionService_CreateOperationsFromPreparedTx(t *testing.T) { operations := service.createOperationsFromPreparedTx(preparedTx) require.Equal(t, expectedOperations, operations) } - -func TestConstructionService_PrepareConstructionOptions(t *testing.T) { - t.Parallel() - - networkProvider := testscommon.NewNetworkProviderMock() - extension := newNetworkProviderExtension(networkProvider) - service := NewConstructionService(networkProvider).(*constructionService) - - t.Run("two operations, no metadata", func(t *testing.T) { - t.Parallel() - - operations := []*types.Operation{ - { - OperationIdentifier: indexToOperationIdentifier(0), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressAlice), - Amount: extension.valueToNativeAmount("-12345"), - }, - { - OperationIdentifier: indexToOperationIdentifier(1), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressBob), - Amount: extension.valueToNativeAmount("12345"), - }, - } - - metadata := make(objectsMap) - - options, err := service.prepareConstructionOptions(operations, metadata) - require.Nil(t, err) - require.Equal(t, opTransfer, options["type"]) - require.Equal(t, testscommon.TestAddressAlice, options["sender"]) - require.Equal(t, testscommon.TestAddressBob, options["receiver"]) - require.Equal(t, "12345", options["value"]) - }) - - t.Run("one operation, with metadata having: receiver", func(t *testing.T) { - t.Parallel() - - operations := []*types.Operation{ - { - OperationIdentifier: indexToOperationIdentifier(0), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressAlice), - Amount: extension.valueToNativeAmount("-12345"), - }, - } - - metadata := make(objectsMap) - metadata["receiver"] = testscommon.TestAddressBob - - options, err := service.prepareConstructionOptions(operations, metadata) - require.Nil(t, err) - require.Equal(t, opTransfer, options["type"]) - require.Equal(t, testscommon.TestAddressAlice, options["sender"]) - require.Equal(t, testscommon.TestAddressBob, options["receiver"]) - require.Equal(t, "12345", options["value"]) - }) - - t.Run("one operation, with metadata having: receiver, value", func(t *testing.T) { - t.Parallel() - - operations := []*types.Operation{ - { - OperationIdentifier: indexToOperationIdentifier(0), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressAlice), - Amount: extension.valueToNativeAmount("ignored"), - }, - } - - metadata := make(objectsMap) - metadata["receiver"] = testscommon.TestAddressBob - metadata["value"] = "12345" - - options, err := service.prepareConstructionOptions(operations, metadata) - require.Nil(t, err) - require.Equal(t, opTransfer, options["type"]) - require.Equal(t, testscommon.TestAddressAlice, options["sender"]) - require.Equal(t, testscommon.TestAddressBob, options["receiver"]) - require.Equal(t, "12345", options["value"]) - }) - - t.Run("one operation, with missing metadata: receiver", func(t *testing.T) { - t.Parallel() - - operations := []*types.Operation{ - { - OperationIdentifier: indexToOperationIdentifier(0), - Type: opTransfer, - Account: addressToAccountIdentifier(testscommon.TestAddressAlice), - Amount: extension.valueToNativeAmount("ignored"), - }, - } - - metadata := make(objectsMap) - - options, err := service.prepareConstructionOptions(operations, metadata) - require.ErrorContains(t, err, "cannot prepare transaction receiver") - require.Nil(t, options) - }) -} From 49ed2d24de4e7735ad17aab15dfa9959bcc07b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Mon, 10 Oct 2022 16:49:51 +0300 Subject: [PATCH 08/12] Add extra unit tests. --- server/services/common_test.go | 56 +++++++++++++++++ server/services/constructionOptions.go | 2 +- server/services/constructionOptions_test.go | 67 +++++++++++++++++++++ server/services/constructionService.go | 2 +- 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 server/services/common_test.go create mode 100644 server/services/constructionOptions_test.go diff --git a/server/services/common_test.go b/server/services/common_test.go new file mode 100644 index 00000000..adfe63db --- /dev/null +++ b/server/services/common_test.go @@ -0,0 +1,56 @@ +package services + +import ( + "math" + "math/big" + "testing" + + "github.com/stretchr/testify/require" +) + +type dummy struct { + A string `json:"a"` + B string `json:"b"` + C uint64 `json:"c"` +} + +func Test_ToObjectsMapAndFromObjectsMap(t *testing.T) { + t.Parallel() + + dummyOriginal := &dummy{ + A: "a", + B: "b", + C: 42, + } + + dummyMap, err := toObjectsMap(dummyOriginal) + require.Nil(t, err) + + dummyConverted := &dummy{} + err = fromObjectsMap(dummyMap, dummyConverted) + require.Nil(t, err) + + require.Equal(t, dummyOriginal, dummyConverted) +} + +func Test_IsZeroAmount(t *testing.T) { + require.True(t, isZeroAmount("")) + require.True(t, isZeroAmount("0")) + require.True(t, isZeroAmount("-0")) + require.False(t, isZeroAmount("1")) + require.False(t, isZeroAmount("-1")) +} + +func Test_GetMagnitudeOfAmount(t *testing.T) { + require.Equal(t, "100", getMagnitudeOfAmount("100")) + require.Equal(t, "100", getMagnitudeOfAmount("-100")) +} + +func Test_MultiplyUint64(t *testing.T) { + require.Equal(t, "340282366920938463426481119284349108225", multiplyUint64(math.MaxUint64, math.MaxUint64).String()) + require.Equal(t, "1", multiplyUint64(1, 1).String()) +} + +func Test_AddBigInt(t *testing.T) { + require.Equal(t, "12", addBigInt(big.NewInt(7), big.NewInt(5)).String()) +} diff --git a/server/services/constructionOptions.go b/server/services/constructionOptions.go index 9c2fee5d..aa5e729c 100644 --- a/server/services/constructionOptions.go +++ b/server/services/constructionOptions.go @@ -45,7 +45,7 @@ func (options *constructionOptions) validate(nativeCurrencySymbol string) error return errors.New("missing option 'sender'") } if len(options.Receiver) == 0 { - return errors.New("missing option 'receive'") + return errors.New("missing option 'receiver'") } if isZeroAmount(options.Amount) { return errors.New("missing option 'amount'") diff --git a/server/services/constructionOptions_test.go b/server/services/constructionOptions_test.go new file mode 100644 index 00000000..6a94dc80 --- /dev/null +++ b/server/services/constructionOptions_test.go @@ -0,0 +1,67 @@ +package services + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestConstructionOptions_CoalesceGasLimit(t *testing.T) { + t.Parallel() + + options := &constructionOptions{ + GasLimit: 80000, + } + require.Equal(t, uint64(80000), options.coalesceGasLimit(50000)) + + options = &constructionOptions{} + require.Equal(t, uint64(50000), options.coalesceGasLimit(50000)) +} + +func TestConstructionOptions_CoalesceGasPrice(t *testing.T) { + t.Parallel() + + options := &constructionOptions{ + GasPrice: 1000000001, + } + require.Equal(t, uint64(1000000001), options.coalesceGasPrice(1000000000)) + + options = &constructionOptions{} + require.Equal(t, uint64(1000000000), options.coalesceGasLimit(1000000000)) +} + +func TestConstructionOptions_Validate(t *testing.T) { + t.Parallel() + + require.ErrorContains(t, (&constructionOptions{}).validate("XeGLD"), "missing option 'sender'") + + require.ErrorContains(t, (&constructionOptions{ + Sender: "alice", + }).validate("XeGLD"), "missing option 'receiver'") + + require.ErrorContains(t, (&constructionOptions{ + Sender: "alice", + Receiver: "bob", + }).validate("XeGLD"), "missing option 'amount'") + + require.ErrorContains(t, (&constructionOptions{ + Sender: "alice", + Receiver: "bob", + Amount: "1234", + }).validate("XeGLD"), "missing option 'currencySymbol'") + + require.ErrorContains(t, (&constructionOptions{ + Sender: "alice", + Receiver: "bob", + Amount: "1234", + CurrencySymbol: "FOO", + Data: []byte("hello"), + }).validate("XeGLD"), "for custom currencies, option 'data' must be empty") + + require.Nil(t, (&constructionOptions{ + Sender: "alice", + Receiver: "bob", + Amount: "1234", + CurrencySymbol: "XeGLD", + }).validate("XeGLD")) +} diff --git a/server/services/constructionService.go b/server/services/constructionService.go index 0b509837..ec002d3a 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -170,7 +170,7 @@ func (service *constructionService) computeData(options *constructionOptions) [] return options.Data } - // TBD: Add implementation for ESDT. + // TODO: Handle in a future PR return make([]byte, 0) } From 3b81e24d320d87bbf92486ddc7eb3e80929a5bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Mon, 10 Oct 2022 17:01:41 +0300 Subject: [PATCH 09/12] Add extra unit tests. --- server/services/constructionMetadata.go | 12 +-- server/services/constructionMetadata_test.go | 82 ++++++++++++++++++++ server/services/constructionOptions.go | 8 +- server/services/constructionOptions_test.go | 8 +- server/services/constructionService_test.go | 8 +- 5 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 server/services/constructionMetadata_test.go diff --git a/server/services/constructionMetadata.go b/server/services/constructionMetadata.go index b54e64b8..b93f0518 100644 --- a/server/services/constructionMetadata.go +++ b/server/services/constructionMetadata.go @@ -66,22 +66,22 @@ func (metadata *constructionMetadata) toTransaction() (*data.Transaction, error) func (metadata *constructionMetadata) validate() error { if len(metadata.Sender) == 0 { - return errors.New("missing metadata: sender") + return errors.New("missing metadata: 'sender'") } if len(metadata.Receiver) == 0 { - return errors.New("missing metadata: receiver") + return errors.New("missing metadata: 'receiver'") } if metadata.GasLimit == 0 { - return errors.New("missing metadata: gasLimit") + return errors.New("missing metadata: 'gasLimit'") } if metadata.GasPrice == 0 { - return errors.New("missing metadata: gasPrice") + return errors.New("missing metadata: 'gasPrice'") } if metadata.Version != 1 { - return errors.New("bad metadata: unexpected version") + return errors.New("bad metadata: unexpected 'version'") } if len(metadata.ChainID) == 0 { - return errors.New("missing metadata: chainID") + return errors.New("missing metadata: 'chainID'") } return nil diff --git a/server/services/constructionMetadata_test.go b/server/services/constructionMetadata_test.go new file mode 100644 index 00000000..f1d4cd9e --- /dev/null +++ b/server/services/constructionMetadata_test.go @@ -0,0 +1,82 @@ +package services + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestConstructionMetadata_ToTransactionJson(t *testing.T) { + t.Parallel() + + options := &constructionMetadata{ + Sender: "alice", + Receiver: "bob", + Nonce: 42, + Amount: "1234", + CurrencySymbol: "XeGLD", + GasLimit: 80000, + GasPrice: 1000000000, + Data: []byte("hello"), + ChainID: "T", + Version: 1, + } + + expectedJson := `{"nonce":42,"value":"1234","receiver":"bob","sender":"alice","gasPrice":1000000000,"gasLimit":80000,"data":"aGVsbG8=","chainID":"T","version":1}` + actualJson, err := options.toTransactionJson() + require.Nil(t, err) + require.Equal(t, expectedJson, string(actualJson)) +} + +func TestConstructionMetadata_Validate(t *testing.T) { + t.Parallel() + + require.ErrorContains(t, (&constructionMetadata{}).validate(), "missing metadata: 'sender'") + + require.ErrorContains(t, (&constructionMetadata{ + Sender: "alice", + }).validate(), "missing metadata: 'receiver'") + + require.ErrorContains(t, (&constructionMetadata{ + Sender: "alice", + Receiver: "bob", + }).validate(), "missing metadata: 'gasLimit'") + + require.ErrorContains(t, (&constructionMetadata{ + Sender: "alice", + Receiver: "bob", + GasLimit: 50000, + }).validate(), "missing metadata: 'gasPrice'") + + require.ErrorContains(t, (&constructionMetadata{ + Sender: "alice", + Receiver: "bob", + GasLimit: 50000, + GasPrice: 1000000000, + }).validate(), "bad metadata: unexpected 'version'") + + require.ErrorContains(t, (&constructionMetadata{ + Sender: "alice", + Receiver: "bob", + GasLimit: 50000, + GasPrice: 1000000000, + Version: 42, + }).validate(), "bad metadata: unexpected 'version'") + + require.ErrorContains(t, (&constructionMetadata{ + Sender: "alice", + Receiver: "bob", + GasLimit: 50000, + GasPrice: 1000000000, + Version: 1, + }).validate(), "missing metadata: 'chainID'") + + require.Nil(t, (&constructionMetadata{ + Sender: "alice", + Receiver: "bob", + GasLimit: 50000, + GasPrice: 1000000000, + Version: 1, + ChainID: "T", + }).validate()) +} diff --git a/server/services/constructionOptions.go b/server/services/constructionOptions.go index aa5e729c..3cdc7140 100644 --- a/server/services/constructionOptions.go +++ b/server/services/constructionOptions.go @@ -42,16 +42,16 @@ func (options *constructionOptions) coalesceGasPrice(minGasPrice uint64) uint64 func (options *constructionOptions) validate(nativeCurrencySymbol string) error { if len(options.Sender) == 0 { - return errors.New("missing option 'sender'") + return errors.New("missing option: 'sender'") } if len(options.Receiver) == 0 { - return errors.New("missing option 'receiver'") + return errors.New("missing option: 'receiver'") } if isZeroAmount(options.Amount) { - return errors.New("missing option 'amount'") + return errors.New("missing option: 'amount'") } if len(options.CurrencySymbol) == 0 { - return errors.New("missing option 'currencySymbol'") + return errors.New("missing option: 'currencySymbol'") } if len(options.Data) > 0 && options.CurrencySymbol != nativeCurrencySymbol { return errors.New("for custom currencies, option 'data' must be empty") diff --git a/server/services/constructionOptions_test.go b/server/services/constructionOptions_test.go index 6a94dc80..15cc269f 100644 --- a/server/services/constructionOptions_test.go +++ b/server/services/constructionOptions_test.go @@ -33,22 +33,22 @@ func TestConstructionOptions_CoalesceGasPrice(t *testing.T) { func TestConstructionOptions_Validate(t *testing.T) { t.Parallel() - require.ErrorContains(t, (&constructionOptions{}).validate("XeGLD"), "missing option 'sender'") + require.ErrorContains(t, (&constructionOptions{}).validate("XeGLD"), "missing option: 'sender'") require.ErrorContains(t, (&constructionOptions{ Sender: "alice", - }).validate("XeGLD"), "missing option 'receiver'") + }).validate("XeGLD"), "missing option: 'receiver'") require.ErrorContains(t, (&constructionOptions{ Sender: "alice", Receiver: "bob", - }).validate("XeGLD"), "missing option 'amount'") + }).validate("XeGLD"), "missing option: 'amount'") require.ErrorContains(t, (&constructionOptions{ Sender: "alice", Receiver: "bob", Amount: "1234", - }).validate("XeGLD"), "missing option 'currencySymbol'") + }).validate("XeGLD"), "missing option: 'currencySymbol'") require.ErrorContains(t, (&constructionOptions{ Sender: "alice", diff --git a/server/services/constructionService_test.go b/server/services/constructionService_test.go index 16aec948..b248ff19 100644 --- a/server/services/constructionService_test.go +++ b/server/services/constructionService_test.go @@ -332,7 +332,7 @@ func TestConstructionService_ConstructionParse(t *testing.T) { extension := newNetworkProviderExtension(networkProvider) service := NewConstructionService(networkProvider) - unsignedTx := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1100000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` + notSignedTx := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1100000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` operations := []*types.Operation{ { @@ -352,7 +352,7 @@ func TestConstructionService_ConstructionParse(t *testing.T) { response, err := service.ConstructionParse(context.Background(), &types.ConstructionParseRequest{ Signed: false, - Transaction: unsignedTx, + Transaction: notSignedTx, }, ) require.Nil(t, err) @@ -366,11 +366,11 @@ func TestConstructionService_ConstructionCombine(t *testing.T) { networkProvider := testscommon.NewNetworkProviderMock() service := NewConstructionService(networkProvider) - unsignedTx := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1100000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` + notSignedTx := `{"nonce":42,"value":"1234","receiver":"erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx","sender":"erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th","gasPrice":1100000000,"gasLimit":57500,"data":"aGVsbG8=","chainID":"T","version":1}` response, err := service.ConstructionCombine(context.Background(), &types.ConstructionCombineRequest{ - UnsignedTransaction: unsignedTx, + UnsignedTransaction: notSignedTx, Signatures: []*types.Signature{ { Bytes: []byte{0xaa, 0xbb}, From dc6a197da5cdbd1c17f9579e3b19701bb8b41b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Mon, 10 Oct 2022 17:57:09 +0300 Subject: [PATCH 10/12] Fix after review. --- server/services/common.go | 11 ++++++++++- server/services/common_test.go | 1 + server/services/constructionMetadata.go | 3 ++- server/services/constructionMetadata_test.go | 4 ++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/server/services/common.go b/server/services/common.go index b399d5ac..18b8356e 100644 --- a/server/services/common.go +++ b/server/services/common.go @@ -38,7 +38,16 @@ func fromObjectsMap(obj objectsMap, value any) error { } func isZeroAmount(amount string) bool { - return amount == "" || amount == "0" || amount == "-0" + if amount == "" { + return true + } + + value, ok := big.NewInt(0).SetString(amount, 10) + if ok { + return value.Sign() == 0 + } + + return false } func getMagnitudeOfAmount(amount string) string { diff --git a/server/services/common_test.go b/server/services/common_test.go index adfe63db..9c17b39d 100644 --- a/server/services/common_test.go +++ b/server/services/common_test.go @@ -37,6 +37,7 @@ func Test_IsZeroAmount(t *testing.T) { require.True(t, isZeroAmount("")) require.True(t, isZeroAmount("0")) require.True(t, isZeroAmount("-0")) + require.True(t, isZeroAmount("00")) require.False(t, isZeroAmount("1")) require.False(t, isZeroAmount("-1")) } diff --git a/server/services/constructionMetadata.go b/server/services/constructionMetadata.go index b93f0518..dc3e4d04 100644 --- a/server/services/constructionMetadata.go +++ b/server/services/constructionMetadata.go @@ -3,6 +3,7 @@ package services import ( "encoding/json" "errors" + "fmt" "github.com/ElrondNetwork/elrond-proxy-go/data" ) @@ -78,7 +79,7 @@ func (metadata *constructionMetadata) validate() error { return errors.New("missing metadata: 'gasPrice'") } if metadata.Version != 1 { - return errors.New("bad metadata: unexpected 'version'") + return fmt.Errorf("bad metadata: unexpected 'version' %v", metadata.Version) } if len(metadata.ChainID) == 0 { return errors.New("missing metadata: 'chainID'") diff --git a/server/services/constructionMetadata_test.go b/server/services/constructionMetadata_test.go index f1d4cd9e..ad6efabf 100644 --- a/server/services/constructionMetadata_test.go +++ b/server/services/constructionMetadata_test.go @@ -53,7 +53,7 @@ func TestConstructionMetadata_Validate(t *testing.T) { Receiver: "bob", GasLimit: 50000, GasPrice: 1000000000, - }).validate(), "bad metadata: unexpected 'version'") + }).validate(), "bad metadata: unexpected 'version' 0") require.ErrorContains(t, (&constructionMetadata{ Sender: "alice", @@ -61,7 +61,7 @@ func TestConstructionMetadata_Validate(t *testing.T) { GasLimit: 50000, GasPrice: 1000000000, Version: 42, - }).validate(), "bad metadata: unexpected 'version'") + }).validate(), "bad metadata: unexpected 'version' 42") require.ErrorContains(t, (&constructionMetadata{ Sender: "alice", From e3ad4e001eb7780cd36817343a7aaa73132f25e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Tue, 11 Oct 2022 13:34:26 +0300 Subject: [PATCH 11/12] Fix after review. --- server/services/constructionMetadata.go | 21 +++++++++++---------- server/services/constructionService.go | 23 ++++++++++++----------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/server/services/constructionMetadata.go b/server/services/constructionMetadata.go index dc3e4d04..efc41b96 100644 --- a/server/services/constructionMetadata.go +++ b/server/services/constructionMetadata.go @@ -51,16 +51,17 @@ func (metadata *constructionMetadata) toTransaction() (*data.Transaction, error) return nil, err } - tx := &data.Transaction{} - tx.Sender = metadata.Sender - tx.Receiver = metadata.Receiver - tx.Nonce = metadata.Nonce - tx.Value = metadata.Amount - tx.GasLimit = metadata.GasLimit - tx.GasPrice = metadata.GasPrice - tx.Data = metadata.Data - tx.ChainID = metadata.ChainID - tx.Version = uint32(metadata.Version) + tx := &data.Transaction{ + Sender: metadata.Sender, + Receiver: metadata.Receiver, + Nonce: metadata.Nonce, + Value: metadata.Amount, + GasLimit: metadata.GasLimit, + GasPrice: metadata.GasPrice, + Data: metadata.Data, + ChainID: metadata.ChainID, + Version: uint32(metadata.Version), + } return tx, nil } diff --git a/server/services/constructionService.go b/server/services/constructionService.go index ec002d3a..49eb6f8a 100644 --- a/server/services/constructionService.go +++ b/server/services/constructionService.go @@ -140,17 +140,18 @@ func (service *constructionService) ConstructionMetadata( return nil, errTyped } - metadata := &constructionMetadata{} - metadata.Nonce = account.Account.Nonce - metadata.Sender = requestOptions.Sender - metadata.Receiver = requestOptions.Receiver - metadata.Amount = requestOptions.Amount - metadata.CurrencySymbol = requestOptions.CurrencySymbol - metadata.GasLimit = gasLimit - metadata.GasPrice = gasPrice - metadata.Data = computedData - metadata.ChainID = service.provider.GetNetworkConfig().NetworkID - metadata.Version = transactionVersion + metadata := &constructionMetadata{ + Nonce: account.Account.Nonce, + Sender: requestOptions.Sender, + Receiver: requestOptions.Receiver, + Amount: requestOptions.Amount, + CurrencySymbol: requestOptions.CurrencySymbol, + GasLimit: gasLimit, + GasPrice: gasPrice, + Data: computedData, + ChainID: service.provider.GetNetworkConfig().NetworkID, + Version: transactionVersion, + } metadataAsObjectsMap, err := toObjectsMap(metadata) if err != nil { From 94721c52cf79e6d13b9f3b85c6dae92b29b2bf1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrei=20B=C4=83ncioiu?= Date: Tue, 11 Oct 2022 13:35:35 +0300 Subject: [PATCH 12/12] Bump version. --- version/constants.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version/constants.go b/version/constants.go index 58fd8fdb..aed8bb06 100644 --- a/version/constants.go +++ b/version/constants.go @@ -5,9 +5,9 @@ const ( RosettaVersion = "v1.4.12" // RosettaMiddlewareVersion is the version of this package (application) - RosettaMiddlewareVersion = "v0.2.8" + RosettaMiddlewareVersion = "v0.3.0" // NodeVersion is the canonical version of the node runtime // TODO: We should fetch this from node/status. - NodeVersion = "v1.3.43-rosetta1" + NodeVersion = "v1.3.44-rosetta1" )