[Access] Add check if tx payer has sufficient amount of flow to pay fee

[illia-malachyn]

Add an invocation of system contract to check if tx payer has enough balance to pay for tx

core contracts PR: onflow/flow-core-contracts#435
emulator PR: onflow/flow-emulator#699

[illia-malachyn]

According to #5823

the execution.ScriptExecutor should be configured in "local only" mode. We do not want scripts to failover to an execution node

Do you know if we still need to check it? We don't use script executor

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 34.55882% with 89 lines in your changes missing coverage. Please review.

Project coverage is 41.52%. Comparing base (9ea6fae) to head (aeee394).
Report is 16 commits behind head on master.

Files	Patch %	Lines
access/mock/blocks.go	0.00%	59 Missing ⚠️
access/validator.go	68.88%	8 Missing and 6 partials ⚠️
cmd/access/node_builder/access_node_builder.go	0.00%	8 Missing ⚠️
access/errors.go	50.00%	3 Missing ⚠️
access/utils/cadence.go	75.00%	1 Missing and 1 partial ⚠️
engine/access/rpc/backend/backend.go	71.42%	1 Missing and 1 partial ⚠️
engine/access/rpc/backend/backend_transactions.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6004      +/-   ##
==========================================
+ Coverage   41.48%   41.52%   +0.03%     
==========================================
  Files        1978     1980       +2     
  Lines      140843   140986     +143     
==========================================
+ Hits        58433    58546     +113     
- Misses      76335    76337       +2     
- Partials     6075     6103      +28     

Flag	Coverage Δ
unittests	41.52% <34.55%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[illia-malachyn]

How to test it manually:

1. Open https://github.com/onflow/flow-go/blob/master/integration/localnet/builder/bootstrap.go#L443

• set --check-payer-balance=true
• reset --script-execution-mode to local-only

2. Open https://github.com/onflow/flow-go/blob/master/cmd/scaffold.go#L1533
   Add flow.Localnet to switch to enable tx fees for local net.

3. Open https://github.com/onflow/flow-go/blob/master/integration/testnet/network.go#L1208
   Add fvm.WithTransactionFee(fvm.DefaultTransactionFees), to the list

4. Build & start local net

cd intergration/localnet
make bootstrap
make start

5. Set up flow project
   flow-c1 init (press No to skip system accounts creation as they're already deployed to the service account)

6. Open the newly created folder with flow.json

7. Update flow.json according to https://github.com/onflow/flow-go/tree/master/integration/localnet#add-service-account-address-and-private-key

8. Create a new account following https://github.com/onflow/flow-go/tree/master/integration/localnet#creating-new-account-on-the-localnet

9. Add previously created account to flow.json
   e.g.

		"alice": {
			"address": "your_address",
			"key": {
				"type": "hex",
				"index": 0,
				"signatureAlgorithm": "ECDSA_P256",
				"hashAlgorithm": "SHA3_256",
				"privateKey": "your_private_key"
			}
		},

10. Create simple_tx.cdc
    transaction { execute {} }

11. Execute transaction on behalf of Alice
    flow-c1 -n localnet transactions send ./simple_tx.cdc --payer alice --authorizer alice --proposer alice

The transaction should fail due to the storage limit check but the fees will be taken anyway thus the account balance will become insufficient.

12. Repeat the transaction
    flow-c1 -n localnet transactions send ./simple_tx.cdc --payer alice --authorizer alice --proposer alice

You should see an error that Alice doesn't have sufficient balance to pay for tx.

13. Create transfer_tokens.cdc transaction

import FungibleToken from 0xee82856bf20e2aa6
import FlowToken from 0x0ae53cb6e3f42a79

transaction(amount: UFix64, to: Address) {

    // The Vault resource that holds the tokens that are being transferred
    let sentVault: @{FungibleToken.Vault}

    prepare(signer: auth(BorrowValue) &Account) {

        // Get a reference to the signer's stored vault
        let vaultRef = signer.storage.borrow<auth(FungibleToken.Withdraw) &FlowToken.Vault>(from: /storage/flowTokenVault)
			?? panic("Could not borrow reference to the owner's Vault!")

        // Withdraw tokens from the signer's stored vault
        self.sentVault <- vaultRef.withdraw(amount: amount)
    }

    execute {

        // Get a reference to the recipient's Receiver
        let receiverRef =  getAccount(to)
            .capabilities.borrow<&{FungibleToken.Receiver}>(/public/flowTokenReceiver)
			?? panic("Could not borrow receiver reference to the recipient's Vault")

        // Deposit the withdrawn tokens in the recipient's receiver
        receiverRef.deposit(from: <-self.sentVault)
    }
}

14. Send some tokens to Alice account
    flow-c1 -n localnet transactions send ./transfer_tokens.cdc "0.01" {ALICE_ACCOUNT_ADDRESS} --payer localnet-service-account --authorizer localnet-service-account --proposer localnet-service-account

15. Execute transaction on behalf of Alice
    flow-c1 -n localnet transactions send ./simple_tx.cdc --payer alice --authorizer alice --proposer alice

You should see no errors. The transaction should be executed successfully.

[illia-malachyn]

@peterargue
I'm running into such behavior during the test: when you execute an empty tx on a newly created account (with default balance), the very first error you run into is a storage limit check that comes from FVM.
If you execute the tx the second time (with less balance as you paid fees for the previous tx), the access node's tx validator will reject the tx as the account balance is insufficient.
This is weird and I assume we want tx validator to fail on the first run too.

Access node's tx validator uses this script to see if balance is sufficient for tx.
But we do fall into an execution node when executing the first transaction.
I assume that FVM has more checks than FlowFees.verifyPayersBalanceForTransactionExecution, it includes a storage limit I believe.
So, we need to adjust the validator to include such checks as well. Otherwise, it won't be very useful on average.

@janezpodhostnik what do you suggest doing in such a case? Is there some cadence code for me to include storage limit check in tx validator's script?

[peterargue]

added a couple last comments. sorry for the confusion on the get balance part. after that this should be ready to go

[janezpodhostnik]

Looks good. I'll approve when the emulator and core contracts are updated.

[peterargue]

@illia-malachyn to get this PR ready to merge please:

1. merge master and fix conflicts
2. remove the replace statement for the core contracts repo, and update to use v1.3.1
3. remove the replace statement for the core contracts repo in your emulator branch
4. Update this PR to point to the new latest commit in your emulator branch
5. Update the emulator branch to point to the new latest commit in this branch

[peterargue]

Looks good. please fix the conflicts again, and we merge tomorrow