search

0xPolygon / polygon-edge

A Framework for Building Ethereum-compatible Blockchain Networks
https://polygon.technology/solutions/polygon-edge/
Apache License 2.0
1.01k stars 534 forks source link

Error: failed sending StakeManager contract deploy transaction: failed to get max priority fee per gas: {"code":-32601,"message":"Method not found"} #1997

Closed 0xDones closed 1 year ago

0xDones commented 1 year ago

Error on polygon-edge polybft stake-manager-deploy

Description

I'm trying to deploy the stake manager contract on my Layer 1 (Besu) with the command polygon-edge polybft stake-manager-deploy but I'm getting the following error:

Error: failed sending StakeManager contract deploy transaction: failed to get max priority fee per gas: {"code":-32601,"message":"Method not found"}

Full command:

    polygon-edge polybft stake-manager-deploy \
        --private-key "${L1_ACCOUNT_PK}" \
        --genesis ${OUT_FOLDER}/genesis.json \
        --jsonrpc ${RPC_URL} \
        --stake-token ${STAKE_TOKEN_ADDRESS} \
        --proxy-contracts-admin ${L2_ACCOUNT_ADDRESS} \
        --json

Can this be related to the new tx pool implementation on Besu? Ref: https://github.com/hyperledger/besu/releases/tag/23.10.0

EDIT:

Tested it on different Besu versions, one with new and another with legacy tx pool. Same error.

Your environment

Stefan-Ethernal commented 1 year ago

The Besu client does not seem to implement eth_maxPriorityFeePerGas, but we need it to properly set maxPriorityFeePerGas when sending a dynamic fee transaction. Can you verify that this is the case?

0xDones commented 1 year ago

@Stefan-Ethernal That sounds about right, I'm not seeing this method listed here in the Besu API reference. I think it's worth mentioning that I'm running a zero gas network on Besu. What are my options? Can I deploy the stake manager contract without using polygon-edge? In this case, any advice?

0xDones commented 1 year ago

@Stefan-Ethernal I assume the same will occur after I run any command that needs to interact with the L1, like polygon-edge rootchain deploy, polygon-edge polybft whitelist-validators, polygon-edge polybft register-validator, polygon-edge polybft stake?

Does it make sense to check for Method Not Found errors here? Because this is what would be returned by Besu in this case, and it's just checking against the following error: ErrTxTypeNotSupported = errors.New("transaction type not supported")

Initially, I was thinking that one way to work around this, for now, would be to translate this function and all the others that call the eth_maxPriorityFeePerGas to some shell instructions and do all of the RPC calls using Foundry. But if my assumption is correct, it would be a lot of work and maybe it would be easier to support the case where eth_maxPriorityFeePerGas is not supported or wait for Besu to support it. Thoughts?

Stefan-Ethernal commented 1 year ago

@refl3ction you are absolutelly right about it: all the bootstrapping commands that are run against the rootchain are sending dynamic fee transactions and are fallbacking to legacy only in case ErrTxTypeNotSupported is retrieved. I was about to propose you to run Foundry and manually deploy all the required contracts, however I agree it is tedious and error prone work.

So I agree with you that we probably can add another error which would fallback to legacy tx sending. It should be an easy win. In fact right now there is a pending task on which colleague is working so I'll let him know about this issue.

0xDones commented 1 year ago

@Stefan-Ethernal Brilliant. That would be much appreciated, this is the only blocker I currently have to deploy the Supernet, if you could let me know once this is pushed, I can test it directly from the source. Thank you!

Stefan-Ethernal commented 1 year ago

@refl3ction Although the PR is still in draft and not merged into the develop (because there might be some additional minor tweaks, in terms of expanding that list), you should be able to test it out. Hope it will work for you. :pray:

Btw I'm not sure when we are going to make another release, but PR should be merged in the following days hopefully.

0xDones commented 1 year ago

@Stefan-Ethernal Awesome! I will give it a try and get back to you soon! Thank you for your quick action on it.

0xDones commented 1 year ago

@Stefan-Ethernal Does this change impact only the bootstrap commands? For example, do I need to run the validators with this change when using Besu as L1 or they would work fine with version 1.3.1?

0xDones commented 1 year ago

@Stefan-Ethernal

The issue is still happening 😢

I noticed the without-london-fork part on the branch name, does the function behave differently if the London fork is enabled? Because in my case it is enabled.

polygon-edge version

[VERSION INFO]
Release version = <none>
Git branch      = EVM-775-check-errors-different-ethereum-clients-return-when-dynamic-fee-tx-is-sent-without-london-fork
Commit hash     = c2dbe4e55499416001dbc288494e2dd86b03b189
Build time      = Thu Oct 19 12:18:27 -03 2023

# deploy stake manager
Error: failed sending StakeManager contract deploy transaction: failed to get max priority fee per gas: {"code":-32601,"message":"Method not found"}
Stefan-Ethernal commented 1 year ago

Ok probably we need to unmarshal JSON response into an Error struct (which is about to be introduced).

I'll probably tackle that tomorrow morning and let you know when it is ready for another round of testing.

Stefan-Ethernal commented 1 year ago

@Stefan-Ethernal Does this change impact only the bootstrap commands? For example, do I need to run the validators with this change when using Besu as L1 or they would work fine with version 1.3.1?

As per this question, checkpoint submission is also affected with this change (aside from stake-manager-deploy, rootchain deploy, stake etc. commands which are interacting with a rootchain)

0xDones commented 1 year ago

@Stefan-Ethernal I left a suggestion for your consideration on the PR, I think the arguments are inverted in the strings.Contains function. I fixed it locally and it worked 🎉

Stefan-Ethernal commented 1 year ago

@refl3ction awesome glad it worked. Thanks for your feedback. 🙂

0xDones commented 1 year ago

@Stefan-Ethernal Ok, so now I was able to execute all of the following commands in the order they appear, without errors:

The first command to fail was polybft stake, any clue?

Error: failed to estimate gas: {"code":-32000,"message":"Execution reverted","data":"0x"}

I think the issue is happening on this call to EstimateGas: https://github.com/0xPolygon/polygon-edge/blob/develop/txrelayer/txrelayer.go#L172-L179

0xDones commented 1 year ago

Could the above error be related to this? https://github.com/0xPolygon/polygon-edge/pull/1998

Stefan-Ethernal commented 1 year ago

Error: failed to estimate gas: {"code":-32000,"message":"Execution reverted","data":"0x"}

This error message often indicates that execution got reverted on the smart contract. Stake transaction can fail because of several reasons:

Can you verify that the contract address provided for the --stake-token flag exists on the rootchain?

Could the above error be related to this? https://github.com/0xPolygon/polygon-edge/pull/1998

I don't think so, because in that case (if an account with 0 balance is executing eth_estimateGas), the given error would be much more like this:

{"jsonrpc":"2.0","id":1,"error":{"code":-32603,"message":"insufficient funds for execution"}}

If you don't figure it out yourself, then please provide the exact commands you are running as well as the genesis.json and ideally debug logs of the Supernets (you can omit private keys of the accounts that are executing given commands, but it would be potentially helpful if you can provide their addresses).

0xDones commented 1 year ago

I regenerated all validator keys and the genesis file and all the commands worked now 🎉 . I still need to start the nodes though, but I'll do it soon.

Questions:

  1. Regarding Supernet ID, the --chain-id argument for the genesis command is for the rootchain ID?
  2. After I ran polygon-edge rootchain deploy, the field supernetID changed from 0 to 1. Is there a way to set this value? What's this value used for?
Stefan-Ethernal commented 1 year ago

Regarding Supernet ID, the --chain-id argument for the genesis command is for the rootchain ID?

Nope, those are two different concepts. Chain ID is an arbitrary number, that uniquely identifies a given chain and its semantics as for Ethereum, to prevent replay attacks. On the other hand, the supernet ID is a unique identifier of the given Supernets chain. It is auto-generated on the StakeManager contract whenever a Supernets gets registered (https://github.com/0xPolygon/core-contracts/blob/7393eef151150177fb3bec8101071a0e87a19f19/contracts/root/staking/StakeManagerChildData.sol#L24-L31), because some time ago it was envisioned that we have a single instance on StakeManager and multiple Supernets chains, which gets registered on the same StakeManager (however still we are not enforcing such architecture, apparently, it seems that we are about to have a separate StakeManager per Supernet).

After I ran polygon-edge rootchain deploy, the field supernetID changed from 0 to 1. Is there a way to set this value? What's this value used for?

Yes, it gets autoincremented each time a new Supernet chain is registered in the StakeManager contract (done with the rootchain deploy command). It is used to uniquely identify Supernet when doing stake, unstake etc., because as I've written previously, it is supported that a single StakeManager serves multiple Supernets.