Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Access] Implemented options to fail or warn when payer is not able to pay #6297

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions access/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,40 @@ func (l *NoopLimiter) IsRateLimited(address flow.Address) bool {
return false
}

type PayerBalanceMode int
Guitarheroua marked this conversation as resolved.
Show resolved Hide resolved

const (
Disabled PayerBalanceMode = iota
WarnCheck
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 +121,7 @@ type TransactionValidationOptions struct {
CheckScriptsParse bool
MaxTransactionByteSize uint64
MaxCollectionByteSize uint64
CheckPayerBalance bool
CheckPayerBalanceMode PayerBalanceMode
}

type ValidationStep struct {
Expand Down Expand Up @@ -115,7 +149,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 +227,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
}
Guitarheroua marked this conversation as resolved.
Show resolved Hide resolved
}

// log and ignore all other errors
Expand Down Expand Up @@ -398,7 +436,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 @@ -172,7 +173,7 @@ type AccessNodeConfig struct {
registerCacheType string
registerCacheSize uint
programCacheSize uint
checkPayerBalance bool
checkPayerBalanceMode string
versionControlEnabled bool
stopControlEnabled bool
}
Expand Down Expand Up @@ -276,7 +277,7 @@ func DefaultAccessNodeConfig() *AccessNodeConfig {
registerCacheType: pstorage.CacheTypeTwoQueue.String(),
registerCacheSize: 0,
programCacheSize: 0,
checkPayerBalance: false,
checkPayerBalanceMode: accessNode.Disabled.String(),
versionControlEnabled: true,
stopControlEnabled: false,
}
Expand Down Expand Up @@ -1414,10 +1415,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",
Guitarheroua marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -1481,7 +1484,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 @@ -1921,6 +1924,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 @@ -1945,7 +1953,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 @@ -111,7 +111,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) {
versionControl: params.VersionControl,
}

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
4 changes: 3 additions & 1 deletion integration/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require (
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 v0.37.7-0.20240830182756-9ac9e1889c34
github.com/onflow/flow-go-sdk v1.0.0-preview.55
github.com/onflow/flow-go/insecure v0.0.0-00010101000000-000000000000
github.com/onflow/flow/protobuf/go/flow v0.4.7
Expand Down Expand Up @@ -359,3 +359,5 @@ require (
replace github.com/onflow/flow-go => ../

replace github.com/onflow/flow-go/insecure => ../insecure

replace github.com/onflow/flow-emulator v1.0.0-preview.41.0.20240829134601-0be55d6970b5 => github.com/The-K-R-O-K/flow-emulator v1.0.0-preview.12.0.20240903103527-a68bcda903d1
Guitarheroua marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions integration/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,8 @@ github.com/Shopify/goreferrer v0.0.0-20181106222321-ec9c9a553398/go.mod h1:a1uqR
github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg=
github.com/StackExchange/wmi v1.2.1 h1:VIkavFPXSjcnS+O8yTq7NI32k0R5Aj+v39y29VYDOSA=
github.com/StackExchange/wmi v1.2.1/go.mod h1:rcmrprowKIVzvc+NUiLncP2uuArMWLCbu9SBzvHz7e8=
github.com/The-K-R-O-K/flow-emulator v1.0.0-preview.12.0.20240903103527-a68bcda903d1 h1:0FHTpn7aY/H0ufDnPhgMvlWcxLtQpVmKS/uwqVVr60I=
github.com/The-K-R-O-K/flow-emulator v1.0.0-preview.12.0.20240903103527-a68bcda903d1/go.mod h1:KJMmuvT7IEZ35GmN1y4RnEi/kCd/DPG+7lvhLVz4mnc=
github.com/VictoriaMetrics/fastcache v1.6.0/go.mod h1:0qHz5QP0GMX4pfmMA/zt5RgfNuXJrTP0zS7DqpHGGTw=
github.com/VictoriaMetrics/fastcache v1.12.1/go.mod h1:tX04vaqcNoQeGLD+ra5pU5sWkuxnzWhEzLwhP9w653o=
github.com/VictoriaMetrics/fastcache v1.12.2 h1:N0y9ASrJ0F6h0QaC3o6uJb3NIZ9VKLjCM7NQbSmF7WI=
Expand Down Expand Up @@ -2153,8 +2155,6 @@ 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-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=
Expand Down
Loading