Skip to content

Commit

Permalink
Merge pull request onflow#6297 from The-K-R-O-K/AndriiSlisarchuk/6139…
Browse files Browse the repository at this point in the history
…-fail-or-warn-tx-balance

[Access] Implemented options to fail or warn when payer is not able to pay
  • Loading branch information
peterargue authored and Guitarheroua committed Oct 9, 2024
1 parent 20b98a3 commit f1c9a94
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 40 deletions.
65 changes: 61 additions & 4 deletions access/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,59 @@ func (l *NoopLimiter) IsRateLimited(address flow.Address) bool {
return false
}

// PayerBalanceMode represents the mode for checking the payer's balance
// when validating transactions. It controls whether and how the balance
// check is performed during transaction validation.
//
// There are few modes available:
//
// - `Disabled` - Balance checking is completely disabled. No checks are
// performed to verify if the payer has sufficient balance to cover the
// transaction fees.
// - `WarnCheck` - Balance is checked, and a warning is logged if the payer
// does not have enough balance. The transaction is still accepted and
// processed regardless of the check result.
// - `EnforceCheck` - Balance is checked, and the transaction is rejected if
// the payer does not have sufficient balance to cover the transaction fees.
type PayerBalanceMode int

const (
// Disabled indicates that payer balance checking is turned off.
Disabled PayerBalanceMode = iota

// WarnCheck logs a warning if the payer's balance is insufficient, but does not prevent the transaction from being accepted.
WarnCheck

// EnforceCheck prevents the transaction from being accepted if the payer's balance is insufficient to cover transaction fees.
EnforceCheck
)

func ParsePayerBalanceMode(s string) (PayerBalanceMode, error) {
switch s {
case Disabled.String():
return Disabled, nil
case WarnCheck.String():
return WarnCheck, nil
case EnforceCheck.String():
return EnforceCheck, nil
default:
return 0, errors.New("invalid payer balance mode")
}
}

func (m PayerBalanceMode) String() string {
switch m {
case Disabled:
return "disabled"
case WarnCheck:
return "warn"
case EnforceCheck:
return "enforce"
default:
return ""
}
}

type TransactionValidationOptions struct {
Expiry uint
ExpiryBuffer uint
Expand All @@ -87,7 +140,7 @@ type TransactionValidationOptions struct {
CheckScriptsParse bool
MaxTransactionByteSize uint64
MaxCollectionByteSize uint64
CheckPayerBalance bool
CheckPayerBalanceMode PayerBalanceMode
}

type ValidationStep struct {
Expand Down Expand Up @@ -115,7 +168,7 @@ func NewTransactionValidator(
options TransactionValidationOptions,
executor execution.ScriptExecutor,
) (*TransactionValidator, error) {
if options.CheckPayerBalance && executor == nil {
if options.CheckPayerBalanceMode != Disabled && executor == nil {
return nil, errors.New("transaction validator cannot use checkPayerBalance with nil executor")
}

Expand Down Expand Up @@ -193,7 +246,11 @@ func (v *TransactionValidator) Validate(ctx context.Context, tx *flow.Transactio
// prevent the transaction from proceeding.
if IsInsufficientBalanceError(err) {
v.transactionValidationMetrics.TransactionValidationFailed(metrics.InsufficientBalance)
return err

if v.options.CheckPayerBalanceMode == EnforceCheck {
log.Warn().Err(err).Str("transactionID", tx.ID().String()).Str("payerAddress", tx.Payer.String()).Msg("enforce check error")
return err
}
}

// log and ignore all other errors
Expand Down Expand Up @@ -398,7 +455,7 @@ func (v *TransactionValidator) checkSignatureFormat(tx *flow.TransactionBody) er
}

func (v *TransactionValidator) checkSufficientBalanceToPayForTransaction(ctx context.Context, tx *flow.TransactionBody) error {
if !v.options.CheckPayerBalance {
if v.options.CheckPayerBalanceMode == Disabled {
return nil
}

Expand Down
37 changes: 23 additions & 14 deletions access/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s *TransactionValidatorSuite) SetupTest() {

s.chain = flow.Testnet.Chain()
s.validatorOptions = access.TransactionValidationOptions{
CheckPayerBalance: true,
CheckPayerBalanceMode: access.EnforceCheck,
MaxTransactionByteSize: flow.DefaultMaxTransactionByteSize,
MaxCollectionByteSize: flow.DefaultMaxCollectionByteSize,
}
Expand Down Expand Up @@ -144,25 +144,34 @@ func (s *TransactionValidatorSuite) TestTransactionValidator_InsufficientBalance

scriptExecutor.
On("ExecuteAtBlockHeight", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(actualResponse, nil).
Once()
Return(actualResponse, nil).Twice()

actualAccountResponse, err := unittest.AccountFixture()
assert.NoError(s.T(), err)
assert.NotNil(s.T(), actualAccountResponse)

validator, err := access.NewTransactionValidator(s.blocks, s.chain, s.metrics, s.validatorOptions, scriptExecutor)
assert.NoError(s.T(), err)
assert.NotNil(s.T(), validator)

txBody := unittest.TransactionBodyFixture()
validateTx := func() error {
txBody := unittest.TransactionBodyFixture()
validator, err := access.NewTransactionValidator(s.blocks, s.chain, s.metrics, s.validatorOptions, scriptExecutor)
assert.NoError(s.T(), err)
assert.NotNil(s.T(), validator)

expectedError := access.InsufficientBalanceError{
Payer: unittest.AddressFixture(),
RequiredBalance: requiredBalance,
return validator.Validate(context.Background(), &txBody)
}

actualErr := validator.Validate(context.Background(), &txBody)

assert.ErrorIs(s.T(), actualErr, expectedError)
s.Run("with enforce check", func() {
err := validateTx()

expectedError := access.InsufficientBalanceError{
Payer: unittest.AddressFixture(),
RequiredBalance: requiredBalance,
}
assert.ErrorIs(s.T(), err, expectedError)
})

s.Run("with warn check", func() {
s.validatorOptions.CheckPayerBalanceMode = access.WarnCheck
err := validateTx()
assert.NoError(s.T(), err)
})
}
24 changes: 16 additions & 8 deletions cmd/access/node_builder/access_node_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"

accessNode "github.com/onflow/flow-go/access"
"github.com/onflow/flow-go/admin/commands"
stateSyncCommands "github.com/onflow/flow-go/admin/commands/state_synchronization"
storageCommands "github.com/onflow/flow-go/admin/commands/storage"
Expand Down Expand Up @@ -171,7 +172,7 @@ type AccessNodeConfig struct {
registerCacheType string
registerCacheSize uint
programCacheSize uint
checkPayerBalance bool
checkPayerBalanceMode string
versionControlEnabled bool
}

Expand Down Expand Up @@ -274,7 +275,7 @@ func DefaultAccessNodeConfig() *AccessNodeConfig {
registerCacheType: pstorage.CacheTypeTwoQueue.String(),
registerCacheSize: 0,
programCacheSize: 0,
checkPayerBalance: false,
checkPayerBalanceMode: accessNode.Disabled.String(),
versionControlEnabled: true,
}
}
Expand Down Expand Up @@ -1402,10 +1403,12 @@ func (builder *FlowAccessNodeBuilder) extraFlags() {
"program-cache-size",
defaultConfig.programCacheSize,
"[experimental] number of blocks to cache for cadence programs. use 0 to disable cache. default: 0. Note: this is an experimental feature and may cause nodes to become unstable under certain workloads. Use with caution.")
flags.BoolVar(&builder.checkPayerBalance,
"check-payer-balance",
defaultConfig.checkPayerBalance,
"checks that a transaction payer has sufficient balance to pay fees before submitting it to collection nodes")

// Payer Balance
flags.StringVar(&builder.checkPayerBalanceMode,
"check-payer-balance-mode",
defaultConfig.checkPayerBalanceMode,
"flag for payer balance validation that specifies whether or not to enforce the balance check. one of [disabled(default), warn, enforce]")
}).ValidateFlags(func() error {
if builder.supportsObserver && (builder.PublicNetworkConfig.BindAddress == cmd.NotSet || builder.PublicNetworkConfig.BindAddress == "") {
return errors.New("public-network-address must be set if supports-observer is true")
Expand Down Expand Up @@ -1469,7 +1472,7 @@ func (builder *FlowAccessNodeBuilder) extraFlags() {
return errors.New("transaction-error-messages-cache-size must be greater than 0")
}

if builder.checkPayerBalance && !builder.executionDataIndexingEnabled {
if builder.checkPayerBalanceMode != accessNode.Disabled.String() && !builder.executionDataIndexingEnabled {
return errors.New("execution-data-indexing-enabled must be set if check-payer-balance is enabled")
}

Expand Down Expand Up @@ -1889,6 +1892,11 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) {
return nil, fmt.Errorf("transaction result query mode 'compare' is not supported")
}

checkPayerBalanceMode, err := accessNode.ParsePayerBalanceMode(builder.checkPayerBalanceMode)
if err != nil {
return nil, fmt.Errorf("could not parse payer balance mode: %w", err)
}

nodeBackend, err := backend.New(backend.Params{
State: node.State,
CollectionRPC: builder.CollectionRPC,
Expand All @@ -1913,7 +1921,7 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) {
TxErrorMessagesCacheSize: builder.TxErrorMessagesCacheSize,
ScriptExecutor: builder.ScriptExecutor,
ScriptExecutionMode: scriptExecMode,
CheckPayerBalance: builder.checkPayerBalance,
CheckPayerBalanceMode: checkPayerBalanceMode,
EventQueryMode: eventQueryMode,
BlockTracker: blockTracker,
SubscriptionHandler: subscription.NewSubscriptionHandler(
Expand Down
14 changes: 10 additions & 4 deletions engine/access/rpc/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type Params struct {
TxErrorMessagesCacheSize uint
ScriptExecutor execution.ScriptExecutor
ScriptExecutionMode IndexQueryMode
CheckPayerBalance bool
CheckPayerBalanceMode access.PayerBalanceMode
EventQueryMode IndexQueryMode
BlockTracker subscription.BlockTracker
SubscriptionHandler *subscription.SubscriptionHandler
Expand Down Expand Up @@ -246,7 +246,13 @@ func New(params Params) (*Backend, error) {
nodeInfo: nodeInfo,
}

txValidator, err := configureTransactionValidator(params.State, params.ChainID, params.AccessMetrics, params.ScriptExecutor, params.CheckPayerBalance)
txValidator, err := configureTransactionValidator(
params.State,
params.ChainID,
params.AccessMetrics,
params.ScriptExecutor,
params.CheckPayerBalanceMode,
)
if err != nil {
return nil, fmt.Errorf("could not create transaction validator: %w", err)
}
Expand Down Expand Up @@ -315,7 +321,7 @@ func configureTransactionValidator(
chainID flow.ChainID,
transactionMetrics module.TransactionValidationMetrics,
executor execution.ScriptExecutor,
checkPayerBalance bool,
checkPayerBalanceMode access.PayerBalanceMode,
) (*access.TransactionValidator, error) {
return access.NewTransactionValidator(
access.NewProtocolStateBlocks(state),
Expand All @@ -330,7 +336,7 @@ func configureTransactionValidator(
MaxGasLimit: flow.DefaultMaxTransactionGasLimit,
MaxTransactionByteSize: flow.DefaultMaxTransactionByteSize,
MaxCollectionByteSize: flow.DefaultMaxCollectionByteSize,
CheckPayerBalance: checkPayerBalance,
CheckPayerBalanceMode: checkPayerBalanceMode,
},
executor,
)
Expand Down
8 changes: 4 additions & 4 deletions integration/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ require (
github.com/onflow/crypto v0.25.2
github.com/onflow/flow-core-contracts/lib/go/contracts v1.3.1
github.com/onflow/flow-core-contracts/lib/go/templates v1.3.1
github.com/onflow/flow-emulator v1.0.0-preview.41.0.20240829134601-0be55d6970b5
github.com/onflow/flow-go v0.37.7-0.20240826193109-e211841b59f5
github.com/onflow/flow-go-sdk v1.0.0-preview.54
github.com/onflow/flow-emulator v1.0.1-0.20240930092334-2f46b2112195
github.com/onflow/flow-go v0.38.0-preview.0
github.com/onflow/flow-go-sdk v1.0.0
github.com/onflow/flow-go/insecure v0.0.0-00010101000000-000000000000
github.com/onflow/flow/protobuf/go/flow v0.4.6
github.com/onflow/flow/protobuf/go/flow v0.4.7
github.com/onflow/go-ethereum v1.14.7
github.com/prometheus/client_golang v1.18.0
github.com/prometheus/client_model v0.5.0
Expand Down
12 changes: 6 additions & 6 deletions integration/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2151,22 +2151,22 @@ github.com/onflow/flow-core-contracts/lib/go/contracts v1.3.1 h1:q9tXLIALwQ76bO4
github.com/onflow/flow-core-contracts/lib/go/contracts v1.3.1/go.mod h1:u/mkP/B+PbV33tEG3qfkhhBlydSvAKxfLZSfB4lsJHg=
github.com/onflow/flow-core-contracts/lib/go/templates v1.3.1 h1:FfhMBAb78p6VAWkJ+iqdKLErGQVQgxk5w6DP5ZruWX8=
github.com/onflow/flow-core-contracts/lib/go/templates v1.3.1/go.mod h1:NgbMOYnMh0GN48VsNKZuiwK7uyk38Wyo8jN9+C9QE30=
github.com/onflow/flow-emulator v1.0.0-preview.41.0.20240829134601-0be55d6970b5 h1:Z5PC3Sqvl2UemY27uwUwzkLb8EXUf+m/aEimxFzOXuc=
github.com/onflow/flow-emulator v1.0.0-preview.41.0.20240829134601-0be55d6970b5/go.mod h1:gFafyGD4Qxs+XT2BRSIjQJ86ILSmgm1VYUoCr1nVxVI=
github.com/onflow/flow-emulator v1.0.1-0.20240930092334-2f46b2112195 h1:buM9uEW5WhFiI9hMDA90lJhokItN1Cmac3ddb0GWSbY=
github.com/onflow/flow-emulator v1.0.1-0.20240930092334-2f46b2112195/go.mod h1:b9gi9kvRfUVHmyz7cTXBsnT12oHOJisvrxpqwtFRMpM=
github.com/onflow/flow-ft/lib/go/contracts v1.0.0 h1:mToacZ5NWqtlWwk/7RgIl/jeKB/Sy/tIXdw90yKHcV0=
github.com/onflow/flow-ft/lib/go/contracts v1.0.0/go.mod h1:PwsL8fC81cjnUnTfmyL/HOIyHnyaw/JA474Wfj2tl6A=
github.com/onflow/flow-ft/lib/go/templates v1.0.0 h1:6cMS/lUJJ17HjKBfMO/eh0GGvnpElPgBXx7h5aoWJhs=
github.com/onflow/flow-ft/lib/go/templates v1.0.0/go.mod h1:uQ8XFqmMK2jxyBSVrmyuwdWjTEb+6zGjRYotfDJ5pAE=
github.com/onflow/flow-go-sdk v1.0.0-M1/go.mod h1:TDW0MNuCs4SvqYRUzkbRnRmHQL1h4X8wURsCw9P9beo=
github.com/onflow/flow-go-sdk v1.0.0-preview.54 h1:5GjCkyIyvE9KolOUUPTkGdEiV/8qOe1MGnLHOLBmthA=
github.com/onflow/flow-go-sdk v1.0.0-preview.54/go.mod h1:u9oFiS25TpnU1EW62PQlq22jzkwBAj4VEiiCBM6nhHo=
github.com/onflow/flow-go-sdk v1.0.0 h1:Ha4fQm1MMKsyaqMkQLCN3rA/yaQKG6DGwiIfx06j40c=
github.com/onflow/flow-go-sdk v1.0.0/go.mod h1:iZkW2IWieVUZKK06mQCxpjJzPDgS0VtGpTaP/rKu6J4=
github.com/onflow/flow-nft/lib/go/contracts v1.2.1 h1:woAAS5z651sDpi7ihAHll8NvRS9uFXIXkL6xR+bKFZY=
github.com/onflow/flow-nft/lib/go/contracts v1.2.1/go.mod h1:2gpbza+uzs1k7x31hkpBPlggIRkI53Suo0n2AyA2HcE=
github.com/onflow/flow-nft/lib/go/templates v1.2.0 h1:JSQyh9rg0RC+D1930BiRXN8lrtMs+ubVMK6aQPon6Yc=
github.com/onflow/flow-nft/lib/go/templates v1.2.0/go.mod h1:p+2hRvtjLUR3MW1NsoJe5Gqgr2eeH49QB6+s6ze00w0=
github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231121210617-52ee94b830c2/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
github.com/onflow/flow/protobuf/go/flow v0.4.6 h1:KE/CsRVfyG5lGBtm1aNcjojMciQyS5GfPF3ixOWRfi0=
github.com/onflow/flow/protobuf/go/flow v0.4.6/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
github.com/onflow/flow/protobuf/go/flow v0.4.7 h1:iP6DFx4wZ3ETORsyeqzHu7neFT3d1CXF6wdK+AOOjmc=
github.com/onflow/flow/protobuf/go/flow v0.4.7/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
github.com/onflow/go-ds-pebble v0.0.0-20240731130313-f186539f382c h1:T0jDCm7k7uqDo26JiiujQ5oryl30itPnlmZQywTu9ng=
github.com/onflow/go-ds-pebble v0.0.0-20240731130313-f186539f382c/go.mod h1:XYnWtulwJvHVOr2B0WVA/UC3dvRgFevjp8Pn9a3E1xo=
github.com/onflow/go-ethereum v1.14.7 h1:gg3awYqI02e3AypRdpJKEvNTJ6kz/OhAqRti0h54Wlc=
Expand Down

0 comments on commit f1c9a94

Please sign in to comment.