From 7df8438916e581dd10da4f9529ae6650e09c605d Mon Sep 17 00:00:00 2001 From: shiatcb Date: Mon, 27 Jun 2022 12:56:06 -0400 Subject: [PATCH] ROSE-420: clean up check:spec logic to avoid duplicate validations --- cmd/check_spec.go | 176 +++------------------------------- cmd/check_spec_utils.go | 203 ++-------------------------------------- 2 files changed, 22 insertions(+), 357 deletions(-) diff --git a/cmd/check_spec.go b/cmd/check_spec.go index fbd04983..fdcda28b 100644 --- a/cmd/check_spec.go +++ b/cmd/check_spec.go @@ -104,100 +104,15 @@ func (cs *checkSpec) networkOptions(ctx context.Context) checkSpecOutput { } defer printInfo("/network/options validated\n") - res, err := cs.offlineFetcher.NetworkOptionsRetry(ctx, Config.Network, nil) + // NetworkOptionsRetry handles validation of /network/options response + // This is an endpoint for offline mode + _, err := cs.offlineFetcher.NetworkOptionsRetry(ctx, Config.Network, nil) if err != nil { printError("%v: unable to fetch network options\n", err.Err) markAllValidationsFailed(output) return output } - // version is required - if res.Version == nil { - setValidationStatus(output, version, checkSpecFailure) - printError("%v: unable to find version in /network/options response\n", errVersionNullPointer) - } - - if err := validateVersion(res.Version.RosettaVersion); err != nil { - setValidationStatus(output, version, checkSpecFailure) - printError("%v\n", err) - } - - if err := validateVersion(res.Version.NodeVersion); err != nil { - setValidationStatus(output, version, checkSpecFailure) - printError("%v\n", err) - } - - // allow is required - if res.Allow == nil { - setValidationStatus(output, allow, checkSpecFailure) - printError("%v: unable to find allow in /network/options response\n", errAllowNullPointer) - } - - if err := validateOperationStatuses(res.Allow.OperationStatuses); err != nil { - setValidationStatus(output, allow, checkSpecFailure) - printError("%v\n", err) - } - - if err := validateOperationTypes(res.Allow.OperationTypes); err != nil { - setValidationStatus(output, allow, checkSpecFailure) - printError("%v\n", err) - } - - if err := validateErrors(res.Allow.Errors); err != nil { - setValidationStatus(output, allow, checkSpecFailure) - printError("%v\n", err) - } - - if err := validateCallMethods(res.Allow.CallMethods); err != nil { - setValidationStatus(output, allow, checkSpecFailure) - printError("%v\n", err) - } - - if err := validateBalanceExemptions(res.Allow.BalanceExemptions); err != nil { - setValidationStatus(output, allow, checkSpecFailure) - printError("%v\n", err) - } - - return output -} - -func (cs *checkSpec) networkStatus(ctx context.Context) checkSpecOutput { - printInfo("validating /network/status ...\n") - output := checkSpecOutput{ - api: networkStatus, - validation: map[checkSpecRequirement]checkSpecStatus{ - currentBlockID: checkSpecSuccess, - currentBlockTime: checkSpecSuccess, - genesisBlockID: checkSpecSuccess, - }, - } - defer printInfo("/network/status validated\n") - - res, err := cs.onlineFetcher.NetworkStatusRetry(ctx, Config.Network, nil) - if err != nil { - printError("%v: unable to fetch network status\n", err.Err) - markAllValidationsFailed(output) - return output - } - - // current_block_identifier is required - if err := validateBlockIdentifier(res.CurrentBlockIdentifier); err != nil { - printError("%v\n", err) - setValidationStatus(output, currentBlockID, checkSpecFailure) - } - - // current_block_timestamp is required - if err := validateTimestamp(res.CurrentBlockTimestamp); err != nil { - printError("%v\n", err) - setValidationStatus(output, currentBlockTime, checkSpecFailure) - } - - // genesis_block_identifier is required - if err := validateBlockIdentifier(res.GenesisBlockIdentifier); err != nil { - printError("%v\n", err) - setValidationStatus(output, genesisBlockID, checkSpecFailure) - } - return output } @@ -213,7 +128,8 @@ func (cs *checkSpec) networkList(ctx context.Context) checkSpecOutput { } defer printInfo("/network/list validated\n") - networks, err := cs.offlineFetcher.NetworkList(ctx, nil) + // endpoint for offline mode + networks, err := cs.offlineFetcher.NetworkListRetry(ctx, nil) if err != nil { printError("%v: unable to fetch network list", err.Err) markAllValidationsFailed(output) @@ -222,7 +138,7 @@ func (cs *checkSpec) networkList(ctx context.Context) checkSpecOutput { if len(networks.NetworkIdentifiers) == 0 { printError("network_identifiers is required") - setValidationStatus(output, networkIDs, checkSpecFailure) + setValidationStatusFailed(output, networkIDs) } for _, network := range networks.NetworkIdentifiers { @@ -232,59 +148,9 @@ func (cs *checkSpec) networkList(ctx context.Context) checkSpecOutput { } } + // static network ID printError("network_identifier in configuration file is not returned by /network/list") - setValidationStatus(output, staticNetworkID, checkSpecFailure) - return output -} - -func (cs *checkSpec) accountBalance(ctx context.Context) checkSpecOutput { - printInfo("validating /account/balance ...\n") - output := checkSpecOutput{ - api: accountBalance, - validation: map[checkSpecRequirement]checkSpecStatus{ - blockID: checkSpecSuccess, - balances: checkSpecSuccess, - }, - } - defer printInfo("/account/balance validated\n") - - acct, partBlockID, currencies, err := cs.getAccount(ctx) - if err != nil { - markAllValidationsFailed(output) - printError("%v: unable to get an account\n", err) - return output - } - if acct == nil { - markAllValidationsFailed(output) - printError("%v\n", errAccountNullPointer) - return output - } - - // fetch account balance - block, amt, _, fetchErr := cs.onlineFetcher.AccountBalanceRetry( - ctx, - Config.Network, - acct, - partBlockID, - currencies) - if err != nil { - markAllValidationsFailed(output) - printError("%v: unable to fetch balance for account: %v\n", fetchErr.Err, *acct) - return output - } - - // block_identifier is required - if err := validateBlockIdentifier(block); err != nil { - printError("%v\n", err) - setValidationStatus(output, blockID, checkSpecFailure) - } - - // balances is required - if err := validateBalances(amt); err != nil { - printError("%v\n", err) - setValidationStatus(output, balances, checkSpecFailure) - } - + setValidationStatusFailed(output, staticNetworkID) return output } @@ -312,7 +178,7 @@ func (cs *checkSpec) accountCoins(ctx context.Context) checkSpecOutput { return output } - block, cs, _, fetchErr := cs.onlineFetcher.AccountCoinsRetry( + _, _, _, fetchErr := cs.onlineFetcher.AccountCoinsRetry( ctx, Config.Network, acct, @@ -323,20 +189,6 @@ func (cs *checkSpec) accountCoins(ctx context.Context) checkSpecOutput { markAllValidationsFailed(output) return output } - - // block_identifier is required - err = validateBlockIdentifier(block) - if err != nil { - printError("%v\n", err) - setValidationStatus(output, blockID, checkSpecFailure) - } - - // coins is required - err = validateCoins(cs) - if err != nil { - printError("%v\n", err) - setValidationStatus(output, coins, checkSpecFailure) - } } return output @@ -380,7 +232,7 @@ func (cs *checkSpec) block(ctx context.Context) checkSpecOutput { block = b } else if !isEqual(types.Hash(*block), types.Hash(*b)) { printError("%v\n", errBlockNotIdempotent) - setValidationStatus(output, idempotent, checkSpecFailure) + setValidationStatusFailed(output, idempotent) } } @@ -388,7 +240,7 @@ func (cs *checkSpec) block(ctx context.Context) checkSpecOutput { res, fetchErr = cs.onlineFetcher.NetworkStatusRetry(ctx, Config.Network, nil) if fetchErr != nil { printError("%v: unable to get network status\n", fetchErr.Err) - setValidationStatus(output, defaultTip, checkSpecFailure) + setValidationStatusFailed(output, defaultTip) return output } tip = res.CurrentBlockIdentifier @@ -398,14 +250,14 @@ func (cs *checkSpec) block(ctx context.Context) checkSpecOutput { block, fetchErr = cs.onlineFetcher.BlockRetry(ctx, Config.Network, emptyBlockID) if fetchErr != nil { printError("%v: unable to fetch tip block\n", fetchErr.Err) - setValidationStatus(output, defaultTip, checkSpecFailure) + setValidationStatusFailed(output, defaultTip) return output } // block index returned from /block should be >= the index returned by /network/status if isNegative(block.BlockIdentifier.Index - tip.Index) { printError("%v\n", errBlockTip) - setValidationStatus(output, defaultTip, checkSpecFailure) + setValidationStatusFailed(output, defaultTip) } return output @@ -514,10 +366,8 @@ func runCheckSpecCmd(_ *cobra.Command, _ []string) error { output := []checkSpecOutput{} // validate api endpoints - output = append(output, cs.networkStatus(ctx)) output = append(output, cs.networkList(ctx)) output = append(output, cs.networkOptions(ctx)) - output = append(output, cs.accountBalance(ctx)) output = append(output, cs.accountCoins(ctx)) output = append(output, cs.block(ctx)) output = append(output, cs.errorObject(ctx)) diff --git a/cmd/check_spec_utils.go b/cmd/check_spec_utils.go index 52d9e4d9..ebbb26cc 100644 --- a/cmd/check_spec_utils.go +++ b/cmd/check_spec_utils.go @@ -19,38 +19,13 @@ import ( "fmt" "github.com/coinbase/rosetta-sdk-go/fetcher" - "github.com/coinbase/rosetta-sdk-go/types" "github.com/fatih/color" ) var ( - errBlockIdentifierNullPointer = errors.New("Null pointer to BlockIdentifier object") - errBlockIdentifierEmptyHash = errors.New("BlockIdentifier can't have empty hash") - errBlockIdentifierNegativeIndex = errors.New("BlockIdentifier can't have negative index") - - errTimestampNegative = errors.New("Timestamp can't be negative") - errVersionEmpty = errors.New("Version can't be empty") - errVersionNullPointer = errors.New("Null pointer to Version object") - - errOperationStatusEmptyStatus = errors.New("OperationStatus can't have empty status value") - errOperationStatusNullPointer = errors.New("Null pointer to OperationStatus object") - errOperationTypeEmpty = errors.New("OperationType can't be empty") - - errErrorEmpty = errors.New("Error object can't be empty") - errErrorEmptyMessage = errors.New("Error object can't have empty message") - errErrorNegativeCode = errors.New("Error object can't have negative code") - - errCallMethodEmptyName = errors.New("Call method name can't be empty") - errBalanceExemptionEmpty = errors.New("Balance exemption can't be empty") - errAllowNullPointer = errors.New("Null pointer to Allow object") - - errBalanceEmptyValue = errors.New("Amount can't be empty") - errCurrencyEmptySymbol = errors.New("Currency can't have empty symbol") - errCurrencyNegativeDecimals = errors.New("Currency can't have negative decimals") - errAccountNullPointer = errors.New("Null pointer to Account object") - - errCoinIdentifierEmpty = errors.New("Coin identifier can't be empty") - errCoinIdentifierNullPointer = errors.New("Null pointer to coin identifier object") + errErrorEmptyMessage = errors.New("Error object can't have empty message") + errErrorNegativeCode = errors.New("Error object can't have negative code") + errAccountNullPointer = errors.New("Null pointer to Account object") errBlockNotIdempotent = errors.New("Multiple calls with the same hash don't return the same block") errBlockTip = errors.New("Unspecified block_identifier doesn't give the tip block") errRosettaConfigNoConstruction = errors.New("No construction element in Rosetta config") @@ -63,8 +38,6 @@ type checkSpecStatus string const ( networkList checkSpecAPI = "/network/list" networkOptions checkSpecAPI = "/network/options" - networkStatus checkSpecAPI = "/network/status" - accountBalance checkSpecAPI = "/account/balance" accountCoins checkSpecAPI = "/account/coins" block checkSpecAPI = "/block" errorObject checkSpecAPI = "error object" @@ -76,12 +49,7 @@ const ( version checkSpecRequirement = "field version is required" allow checkSpecRequirement = "field allow is required" - currentBlockID checkSpecRequirement = "current_block_identifier is required" - currentBlockTime checkSpecRequirement = "current_block_timestamp is required" - genesisBlockID checkSpecRequirement = "genesis_block_identifier is required" - blockID checkSpecRequirement = "block_identifier is required" - balances checkSpecRequirement = "field balances is required" coins checkSpecRequirement = "field coins is required" idempotent checkSpecRequirement = "same hash should return the same block" defaultTip checkSpecRequirement = "tip should be returned if block_identifier is not specified" @@ -99,159 +67,6 @@ type checkSpecOutput struct { validation map[checkSpecRequirement]checkSpecStatus } -func validateBlockIdentifier(blockID *types.BlockIdentifier) error { - if blockID == nil { - return errBlockIdentifierNullPointer - } - - if isEmpty(blockID.Hash) { - return errBlockIdentifierEmptyHash - } - - if isNegative(blockID.Index) { - return errBlockIdentifierNegativeIndex - } - - return nil -} - -func validateTimestamp(time int64) error { - if isNegative(time) { - return errTimestampNegative - } - - return nil -} - -func validateVersion(version string) error { - if isEmpty(version) { - return errVersionEmpty - } - - return nil -} - -func validateOperationStatuses(oss []*types.OperationStatus) error { - for _, os := range oss { - if os == nil { - return errOperationStatusNullPointer - } - - if isEmpty(os.Status) { - return errOperationStatusEmptyStatus - } - } - - return nil -} - -func validateOperationTypes(ots []string) error { - for _, ot := range ots { - if isEmpty(ot) { - return errOperationTypeEmpty - } - } - - return nil -} - -func validateErrors(errors []*types.Error) error { - for _, err := range errors { - if err == nil { - return errErrorEmpty - } - - if isNegative(int64(err.Code)) { - return errErrorNegativeCode - } - - if isEmpty(err.Message) { - return errErrorEmptyMessage - } - } - - return nil -} - -func validateCallMethods(methods []string) error { - for _, m := range methods { - if isEmpty(m) { - return errCallMethodEmptyName - } - } - - return nil -} - -func validateBalanceExemptions(exs []*types.BalanceExemption) error { - for _, e := range exs { - if e == nil { - return errBalanceExemptionEmpty - } - } - - return nil -} - -func validateBalances(balances []*types.Amount) error { - for _, b := range balances { - if err := validateBalance(b); err != nil { - return err - } - } - - return nil -} - -func validateBalance(amt *types.Amount) error { - if isEmpty(amt.Value) { - return errBalanceEmptyValue - } - - if err := validateCurrency(amt.Currency); err != nil { - return err - } - - return nil -} - -func validateCurrency(currency *types.Currency) error { - if isEmpty(currency.Symbol) { - return errCurrencyEmptySymbol - } - - if isNegative(int64(currency.Decimals)) { - return errCurrencyNegativeDecimals - } - - return nil -} - -func validateCoins(coins []*types.Coin) error { - for _, coin := range coins { - if err := validateCoinIdentifier(coin.CoinIdentifier); err != nil { - return err - } - if err := validateBalance(coin.Amount); err != nil { - return err - } - } - - return nil -} - -func validateCoinIdentifier(coinID *types.CoinIdentifier) error { - if coinID == nil { - return errCoinIdentifierNullPointer - } - - if isEmpty(coinID.Identifier) { - return errCoinIdentifierEmpty - } - - return nil -} - func twoModes() checkSpecOutput { output := checkSpecOutput{ api: modes, @@ -263,7 +78,7 @@ func twoModes() checkSpecOutput { if isEmpty(Config.OnlineURL) || isEmpty(Config.Construction.OfflineURL) || isEqual(Config.OnlineURL, Config.Construction.OfflineURL) { - setValidationStatus(output, diffURLs, checkSpecFailure) + setValidationStatusFailed(output, diffURLs) } return output @@ -275,20 +90,20 @@ func markAllValidationsFailed(output checkSpecOutput) { } } -func setValidationStatus(output checkSpecOutput, req checkSpecRequirement, status checkSpecStatus) { - output.validation[req] = status +func setValidationStatusFailed(output checkSpecOutput, req checkSpecRequirement) { + output.validation[req] = checkSpecFailure } func validateErrorObject(err *fetcher.Error, output checkSpecOutput) { if err != nil { if err.ClientErr != nil && isNegative(int64(err.ClientErr.Code)) { printError("%v\n", errErrorNegativeCode) - setValidationStatus(output, errorCode, checkSpecFailure) + setValidationStatusFailed(output, errorCode) } if err.ClientErr != nil && isEmpty(err.ClientErr.Message) { printError("%v\n", errErrorEmptyMessage) - setValidationStatus(output, errorMessage, checkSpecFailure) + setValidationStatusFailed(output, errorMessage) } } } @@ -315,7 +130,7 @@ func printValidationResult(format string, status checkSpecStatus, a ...interface func printCheckSpecOutputHeader() { printInfo("%v\n", "+--------------------------+-------------------------------------------------------------------+-----------+") - printInfo("%v\n", "| API | Requirement | Status |") + printInfo("%v\n", "| API | Requirement | Status |") printInfo("%v\n", "+--------------------------+-------------------------------------------------------------------+-----------+") }