Documentation is not clear about provider context manager [APE-618]

PoCk3T-SPAI commented 1 year ago

Environment information

OS: macOS / linux / win
Python Version: x.x.x
ape and plugin versions:

Environment information

apeworx/ape:latest Docker image

$ ape --version
0.6.2

$ ape plugins list
Installed Plugins:
  tokens       0.6.0
  alchemy      0.6.0
  trezor       0.6.0
  ledger       0.6.0
  ens          0.6.0
  infura       0.6.0
  hardhat      0.6.0
  etherscan    0.6.0
  foundry      0.6.0
  vyper        0.6.1
  avalanche    0.1.0a3
  solidity     0.6.0
  bsc          0.6.0
  polygon      0.6.0
  template     0.6.0

Contents of your ape-config.yaml (NOTE: do not post anything private like RPC urls or secrets!):

$ cat ape-config.yaml
dependencies:
  - name: OpenZeppelin
    github: OpenZeppelin/openzeppelin-contracts
    version: 4.8.1    

  - name: Chainlink
    github: smartcontractkit/chainlink
    branch: master
    contracts_folder: contracts/src/v0.8

solidity:
  import_remapping:
    - "@openzeppelin=OpenZeppelin/4.8.1"
    - "@chainlink=Chainlink/master"

geth:
  bsc:
    testnet:
      uri: https://bsc-testnet.public.blastapi.io 
    mainnet:
      uri: https://bsc-mainnet.public.blastapi.io 
  avalanche:
    fuji:
      uri: https://api.avax-test.network/ext/bc/C/rpc # https://ava-testnet.public.blastapi.io/ext/bc/C/rpc
    mainnet:
      uri: https://api.avax.network/ext/bc/C/rpc # https://ava-mainnet.public.blastapi.io/ext/bc/C/rpc
  fantom:
    testnet:
      uri: https://fantom-testnet.public.blastapi.io
    opera:
      uri: https://fantom-mainnet.public.blastapi.io
  polygon:
    mumbai:
      uri: https://polygon-mumbai.g.alchemy.com # https://polygon-testnet.public.blastapi.io
    mainnet:
      uri: https://polygon-mainnet.public.blastapi.io

What went wrong?

In ape console:

In [16]: with networks.polygon.mumbai.use_provider("geth") as provider:
    ...:         provider.connect()
    ...:
WARNING: Connecting Geth plugin to non-Geth client 'bor'.
WARNING: Connecting Geth plugin to non-Geth client 'bor'.

In [32]: provider.get_balance(account.address)
Out[32]: 2000000000000000000

In [33]: account.balance
Out[33]: 0

In [38]: contract = account.deploy(project.SpiderwebPredictionRequest, publish=True)
File ~/.local/lib/python3.9/site-packages/ape/api/accounts.py:276, in AccountAPI.prepare_transaction(self, txn)
    273 txn = self.provider.prepare_transaction(txn)
    275 **if txn.total_transfer_value > self.balance:**
--> 276     raise AccountsError(
    277         "Transfer value meets or exceeds account balance.\n"
    278         "Are you using the correct provider/account combination?\n"
    279         f"(transfer_value={txn.total_transfer_value}, balance={self.balance})."
    280     )
    282 return txn

AccountsError: Transfer value meets or exceeds account balance.
Are you using the correct provider/account combination?
(transfer_value=1000000000, balance=0).

Problem 1 = Both out[32] and out[33] should match, but unfortunately it doesn't, thus, Ape thinks that my wallet is not funded enough for my smart contract deployment, which ends up failing with

ape.exceptions.AccountsError: Transfer value meets or _exceeds_ account balance.
Are you using the correct provider/account combination?
(transfer_value=1000000000, balance=0).

Problem 2 = as per the source code seeable in the trace above, the IF checks for the balance to be superior or equal to the transfer value (makes sense), so the error message should be Transfer value meets or _be inferior to_ account balance.

How can it be fixed?

Problem 1 -> is there any way to force a refresh of the balance? something like account.set_balance(provider.get_balance(account.address))

Problem 2 -> typo fix

Hope I'm not doing anything wrong? :)

PoCk3T-SPAI commented 1 year ago

Answering myself, hope it helps any other (beginners) Web3 devs in the future: Unlike one could understand from the doc, if using a custom ecosystem/blockchain/network, all subsequent commands needs to stay within the with ... as provider block:

with networks.polygon.mumbai.use_provider("geth") as provider:
    provider.connect()
    account.deploy(project.MyContract, publish=True)

Problem #2 remains valid (it's just a typo though, so I'm closing anyway)

fubuloubu commented 1 year ago

Answering myself, hope it helps any other (beginners) Web3 devs in the future: Unlike one could understand from the doc, if using a custom ecosystem/blockchain/network, all subsequent commands needs to stay within the with ... as provider block:
with networks.polygon.mumbai.use_provider("geth") as provider:
    provider.connect()
    account.deploy(project.MyContract, publish=True)

btw, you shouldn't have to do provider.connect() as the context manager will call this automatically (it doesn't hurt though with that provider since it doesn't start anything)

fubuloubu commented 1 year ago

Yes, the documentation needs to be improved to show that the reference to provider after the context manager is no longer the primary connected provider, but the reference will still work (e.g. you can still make a call to provider.get_balance, but account.balance is making a call to the current provider in the connection stack, or basically provider != networks.provider after you exit the context manager)

PoCk3T-SPAI commented 1 year ago

Thank you for the feedbacks @fubuloubu , very insightful Do you want me to try and open a PR for Transfer value meets or _be inferior to_ account balance in lieu of Transfer value meets or _exceeds_ account balance. or did I miss something also here?

fubuloubu commented 1 year ago

Thank you for the feedbacks @fubuloubu , very insightful Do you want me to try and open a PR for Transfer value meets or _be inferior to_ account balance in lieu of Transfer value meets or _exceeds_ account balance. or did I miss something also here?

I don't think that the error message is incorrectly worded, but I do agree it could be clearer/more direct. Overall, docs issues with people understanding how the provider context manager works is the main problem

antazoey commented 1 year ago

Note: it should not matter if using parse_network_choice() versus the getattr approach. They do the same thing.

antazoey commented 1 year ago

I am guessing there was a bug in here somewhere, maybe it is fixed now. The double connect() call could have causes an issue, I don' know. ProviderContextManager is completely missing from the user guides, I am adding that now.

What I want to do is after the next release, let's start over and we will fix anything remaining issues.

ApeWorX / ape