Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

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

Closed
0xDones opened this issue Oct 18, 2023 · 18 comments · Fixed by #2000
Assignees
Labels
info needed More information needed

Comments

@0xDones
Copy link

0xDones commented Oct 18, 2023

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

  • MacOs
  • polygon-edge 1.3.1
  • besu 23.10.0
@Stefan-Ethernal
Copy link
Collaborator

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
Copy link
Author

0xDones commented Oct 19, 2023

@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
Copy link
Author

0xDones commented Oct 19, 2023

@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
Copy link
Collaborator

@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
Copy link
Author

0xDones commented Oct 19, 2023

@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
Copy link
Collaborator

Stefan-Ethernal commented Oct 19, 2023

@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. 🙏

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

@0xDones
Copy link
Author

0xDones commented Oct 19, 2023

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

@0xDones
Copy link
Author

0xDones commented Oct 19, 2023

@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
Copy link
Author

0xDones commented Oct 19, 2023

@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
Copy link
Collaborator

Stefan-Ethernal commented Oct 19, 2023

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
Copy link
Collaborator

@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
Copy link
Author

0xDones commented Oct 19, 2023

@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
Copy link
Collaborator

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

@0xDones
Copy link
Author

0xDones commented Oct 19, 2023

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

  • polybft stake-manager-deploy
  • rootchain deploy
  • polybft whitelist-validators
  • polybft register-validator

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
Copy link
Author

0xDones commented Oct 19, 2023

Could the above error be related to this? #1998

@Stefan-Ethernal
Copy link
Collaborator

Stefan-Ethernal commented Oct 20, 2023

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:

  • provided supernet id is invalid,
  • staking token address was not provided or
  • that an account does not have enough balance of the given staking token

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? #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).

@Stefan-Ethernal Stefan-Ethernal added the info needed More information needed label Oct 20, 2023
@0xDones
Copy link
Author

0xDones commented Oct 20, 2023

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
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
info needed More information needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants