From b8d2e662b65f3d4426f8519fabe7d1b78d3dabd2 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Fri, 5 May 2023 16:09:40 +0800 Subject: [PATCH 1/6] fix nonce error for debug trace api (#256) --- core/types/transaction.go | 2 ++ eth/api_tracer.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/core/types/transaction.go b/core/types/transaction.go index 25942e7e3..127ffed50 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -726,3 +726,5 @@ func (m Message) Gas() uint64 { return m.gasLimit } func (m Message) Nonce() uint64 { return m.nonce } func (m Message) Data() []byte { return m.data } func (m Message) CheckNonce() bool { return m.checkNonce } + +func (m *Message) SetNonce(nonce uint64) { m.nonce = nonce } diff --git a/eth/api_tracer.go b/eth/api_tracer.go index 3ab764ac0..87c9308fd 100644 --- a/eth/api_tracer.go +++ b/eth/api_tracer.go @@ -474,6 +474,8 @@ func (api *PrivateDebugAPI) traceBlock(ctx context.Context, block *types.Block, } // Generate the next state snapshot fast without tracing msg, _ := tx.AsMessage(signer, balacne, block.Number()) + // Set nonce to fix issue #256 + msg.SetNonce(statedb.GetNonce(*tx.From())) vmctx := core.NewEVMContext(msg, block.Header(), api.eth.blockchain, nil) vmenv := vm.NewEVM(vmctx, statedb, XDCxState, api.config, vm.Config{}) From 4678312cbf85178399e6524131f9f8b1cf3ac5cf Mon Sep 17 00:00:00 2001 From: Banana-J Date: Sat, 5 Aug 2023 22:17:26 +1000 Subject: [PATCH 2/6] Add pull request template --- .github/pull_request_template.md | 39 ++++++++++++++++++++++++++++++++ .gitignore | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..e3a182057 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,39 @@ +## Proposed changes + +Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue(or XIP) or explain it in the pull request. +You description should at least include below key points: +- Proposed version bump. i.e Major, Minor or patch +- Proposed release date to testnet and mainnet + +## Types of changes + +What types of changes does your code introduce to XDC network? +_Put an `x` in the boxes that apply_ + +- [ ] Bugfix (non-breaking change which fixes an issue) +- [ ] New feature (non-breaking change which adds functionality) +- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) +- [ ] Documentation Update (if none of the other choices apply) +- [ ] Regular KTLO or any of the maintaince work. e.g code style + +## Impacted Components + +Which part of the codebase this PR will touch base on, +_Put an `x` in the boxes that apply_ + +- [ ] Consensus +- [ ] Geth +- [ ] Network +- [ ] Smart Contract +- [ ] EVM +- [ ] Account +- [ ] Not sure + +## Checklist +_Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._ + +- [ ] This PR have sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage +- [ ] Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet. +- [ ] Tested the backwards compatibility. +- [ ] Tested with XDC nodes running this version co-exist with those running the previous version. +- [ ] Relevant documentation has been updated as part of this PR \ No newline at end of file diff --git a/.gitignore b/.gitignore index f34305b08..36a97a3c0 100644 --- a/.gitignore +++ b/.gitignore @@ -56,4 +56,4 @@ profile.cov **/yarn-error.log coverage.txt -go.sum \ No newline at end of file +go.sum From 7b58b851ebf6f980603b5fd4239d156a4b969f1f Mon Sep 17 00:00:00 2001 From: Banana-J Date: Mon, 7 Aug 2023 20:04:15 +1000 Subject: [PATCH 3/6] update the pull request template (#306) --- .github/pull_request_template.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index e3a182057..3f54855eb 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -8,18 +8,20 @@ You description should at least include below key points: ## Types of changes What types of changes does your code introduce to XDC network? -_Put an `x` in the boxes that apply_ +_Put an `✅` in the boxes that apply_ - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation Update (if none of the other choices apply) - [ ] Regular KTLO or any of the maintaince work. e.g code style +- [ ] Other (If none of the above is relevant, please specify reasons below) + ## Impacted Components Which part of the codebase this PR will touch base on, -_Put an `x` in the boxes that apply_ +_Put an `✅` in the boxes that apply_ - [ ] Consensus - [ ] Geth @@ -27,12 +29,13 @@ _Put an `x` in the boxes that apply_ - [ ] Smart Contract - [ ] EVM - [ ] Account -- [ ] Not sure +- [ ] External components +- [ ] Not sure (Please specify below) ## Checklist -_Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._ +_Put an `✅` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._ -- [ ] This PR have sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage +- [ ] This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage - [ ] Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet. - [ ] Tested the backwards compatibility. - [ ] Tested with XDC nodes running this version co-exist with those running the previous version. From 066adb5d81ce3adad52e7bddb55b277718da3ec9 Mon Sep 17 00:00:00 2001 From: Banana-J Date: Sun, 13 Aug 2023 09:31:26 +1000 Subject: [PATCH 4/6] Make the pull request template a bit shorter (#308) --- .github/pull_request_template.md | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3f54855eb..c03200b89 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,42 +1,33 @@ -## Proposed changes - -Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue(or XIP) or explain it in the pull request. -You description should at least include below key points: -- Proposed version bump. i.e Major, Minor or patch -- Proposed release date to testnet and mainnet +# Proposed changes +Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. ## Types of changes What types of changes does your code introduce to XDC network? _Put an `✅` in the boxes that apply_ -- [ ] Bugfix (non-breaking change which fixes an issue) -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) -- [ ] Documentation Update (if none of the other choices apply) -- [ ] Regular KTLO or any of the maintaince work. e.g code style -- [ ] Other (If none of the above is relevant, please specify reasons below) - +- [ ] Bugfix +- [ ] New feature +- [ ] Documentation Update or any other KTLO +- [ ] Not sure (Please specify below) ## Impacted Components - Which part of the codebase this PR will touch base on, + _Put an `✅` in the boxes that apply_ - [ ] Consensus -- [ ] Geth +- [ ] Account - [ ] Network +- [ ] Geth - [ ] Smart Contract -- [ ] EVM -- [ ] Account - [ ] External components - [ ] Not sure (Please specify below) ## Checklist -_Put an `✅` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._ +_Put an `✅` in the boxes once you have confirmed below actions (or provide reasons on not doing so) that_ - [ ] This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage - [ ] Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet. - [ ] Tested the backwards compatibility. -- [ ] Tested with XDC nodes running this version co-exist with those running the previous version. - [ ] Relevant documentation has been updated as part of this PR \ No newline at end of file From 228a26f53e929d22d6c9b32b72a459fedfb5a0af Mon Sep 17 00:00:00 2001 From: Liam Date: Fri, 25 Aug 2023 23:53:28 +1000 Subject: [PATCH 5/6] Merge pull request #312 from XinFinOrg/dev-upgrade CI Upgrade --- .github/pull_request_template.md | 14 +++++++++----- cicd/.dockerignore | 1 + consensus/XDPoS/XDPoS.go | 3 +-- consensus/XDPoS/engines/engine_v2/utils.go | 6 ++++++ internal/ethapi/api.go | 3 +++ params/config.go | 13 ++++++------- 6 files changed, 26 insertions(+), 14 deletions(-) create mode 100644 cicd/.dockerignore diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index c03200b89..151a3fabe 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,10 +6,12 @@ Describe the big picture of your changes here to communicate to the maintainers What types of changes does your code introduce to XDC network? _Put an `✅` in the boxes that apply_ -- [ ] Bugfix -- [ ] New feature -- [ ] Documentation Update or any other KTLO -- [ ] Not sure (Please specify below) +- [ ] Bugfix (non-breaking change which fixes an issue) +- [ ] New feature (non-breaking change which adds functionality) +- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) +- [ ] Documentation Update (if none of the other choices apply) +- [ ] Regular KTLO or any of the maintaince work. e.g code style +- [ ] CICD Improvement ## Impacted Components Which part of the codebase this PR will touch base on, @@ -30,4 +32,6 @@ _Put an `✅` in the boxes once you have confirmed below actions (or provide rea - [ ] This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage - [ ] Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet. - [ ] Tested the backwards compatibility. -- [ ] Relevant documentation has been updated as part of this PR \ No newline at end of file +- [ ] Tested with XDC nodes running this version co-exist with those running the previous version. +- [ ] Relevant documentation has been updated as part of this PR +- [ ] N/A diff --git a/cicd/.dockerignore b/cicd/.dockerignore new file mode 100644 index 000000000..1d1fe94df --- /dev/null +++ b/cicd/.dockerignore @@ -0,0 +1 @@ +Dockerfile \ No newline at end of file diff --git a/consensus/XDPoS/XDPoS.go b/consensus/XDPoS/XDPoS.go index ebb7bfaf7..b0a49b2ee 100644 --- a/consensus/XDPoS/XDPoS.go +++ b/consensus/XDPoS/XDPoS.go @@ -315,8 +315,7 @@ func (x *XDPoS) GetSnapshot(chain consensus.ChainReader, header *types.Header) ( } func (x *XDPoS) GetAuthorisedSignersFromSnapshot(chain consensus.ChainReader, header *types.Header) ([]common.Address, error) { - // Legacy V1 function - return []common.Address{}, nil + return x.EngineV2.GetSignersFromSnapshot(chain, header) } func (x *XDPoS) FindParentBlockToAssign(chain consensus.ChainReader, currentBlock *types.Block) *types.Block { diff --git a/consensus/XDPoS/engines/engine_v2/utils.go b/consensus/XDPoS/engines/engine_v2/utils.go index 7e396c2d5..bc2524727 100644 --- a/consensus/XDPoS/engines/engine_v2/utils.go +++ b/consensus/XDPoS/engines/engine_v2/utils.go @@ -6,6 +6,7 @@ import ( "github.com/XinFinOrg/XDC-Subnet/accounts" "github.com/XinFinOrg/XDC-Subnet/common" "github.com/XinFinOrg/XDC-Subnet/consensus/XDPoS/utils" + "github.com/XinFinOrg/XDC-Subnet/consensus" "github.com/XinFinOrg/XDC-Subnet/core/types" "github.com/XinFinOrg/XDC-Subnet/crypto" "github.com/XinFinOrg/XDC-Subnet/crypto/sha3" @@ -157,3 +158,8 @@ func (x *XDPoS_v2) GetRoundNumber(header *types.Header) (types.Round, error) { return decodedExtraField.Round, nil } } + +func (x *XDPoS_v2) GetSignersFromSnapshot(chain consensus.ChainReader, header *types.Header) ([]common.Address, error) { + snap, err := x.getSnapshot(chain, header.Number.Uint64(), false) + return snap.NextEpochMasterNodes, err +} diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index dc1b79ce8..9daf5875f 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -2967,6 +2967,9 @@ func GetSignersFromBlocks(b Backend, blockNumber uint64, blockHash common.Hash, return addrs, err } blockData, err := b.BlockByNumber(nil, rpc.BlockNumber(i)) + if err != nil { + return addrs, err + } signTxs := engine.CacheSigningTxs(header.Hash(), blockData.Transactions()) for _, signtx := range signTxs { blkHash := common.BytesToHash(signtx.Data()[len(signtx.Data())-32:]) diff --git a/params/config.go b/params/config.go index ebd9ea712..bf335dbe4 100644 --- a/params/config.go +++ b/params/config.go @@ -52,9 +52,9 @@ var ( TestnetV2Configs = map[uint64]*V2Config{ Default: { SwitchRound: 0, - CertThreshold: 3, - TimeoutSyncThreshold: 2, - TimeoutPeriod: 4, + CertThreshold: 7, + TimeoutSyncThreshold: 3, + TimeoutPeriod: 60, MinePeriod: 2, }, } @@ -159,10 +159,9 @@ var ( Gap: 450, FoudationWalletAddr: common.HexToAddress("xdc746249c61f5832c5eed53172776b460491bdcd5c"), V2: &V2{ - SwitchBlock: common.TIPV2SwitchBlock, - CurrentConfig: TestnetV2Configs[0], - AllConfigs: TestnetV2Configs, - SkipV2Validation: true, + SwitchBlock: common.TIPV2SwitchBlock, + CurrentConfig: TestnetV2Configs[0], + AllConfigs: TestnetV2Configs, }, }, } From 34fbba1c5656e6ae0694e842944e797d59ed770a Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Thu, 27 Oct 2022 17:25:25 +0800 Subject: [PATCH 6/6] internal/ethapi: EstimateGas and Call handle revert error(#173) (#200) hot fix for EstimateGas and Call handle revert error https://github.com/XinFinOrg/XDPoSChain/issues/173 --- accounts/abi/abi.go | 26 ++++++ accounts/abi/argument.go | 12 +++ accounts/abi/bind/backends/simulated.go | 5 +- core/state_processor.go | 2 +- core/state_transition.go | 12 +-- core/token_validator.go | 2 +- eth/api_tracer.go | 4 +- eth/tracers/tracers_test.go | 4 +- internal/ethapi/api.go | 116 ++++++++++++++++++++---- les/odr_test.go | 4 +- light/odr_test.go | 2 +- tests/state_test_util.go | 2 +- 12 files changed, 153 insertions(+), 38 deletions(-) diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index 254b1f7fb..d94e2ed1a 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -19,8 +19,11 @@ package abi import ( "bytes" "encoding/json" + "errors" "fmt" "io" + + "github.com/XinFinOrg/XDC-Subnet/crypto" ) // The ABI holds information about a contract's context and available @@ -144,3 +147,26 @@ func (abi *ABI) MethodById(sigdata []byte) (*Method, error) { } return nil, fmt.Errorf("no method with id: %#x", sigdata[:4]) } + + +// revertSelector is a special function selector for revert reason unpacking. +var revertSelector = crypto.Keccak256([]byte("Error(string)"))[:4] + +// UnpackRevert resolves the abi-encoded revert reason. According to the solidity +// spec https://solidity.readthedocs.io/en/latest/control-structures.html#revert, +// the provided revert reason is abi-encoded as if it were a call to a function +// `Error(string)`. So it's a special tool for it. +func UnpackRevert(data []byte) (string, error) { + if len(data) < 4 { + return "", errors.New("invalid data for unpacking") + } + if !bytes.Equal(data[:4], revertSelector) { + return "", errors.New("invalid data for unpacking") + } + typ, _ := NewType("string") + unpacked, err := (Arguments{{Type: typ}}).Unpack2(data[4:]) + if err != nil { + return "", err + } + return unpacked[0].(string), nil +} diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index 512d8fdfa..a45885c94 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -18,6 +18,7 @@ package abi import ( "encoding/json" + "errors" "fmt" "reflect" "strings" @@ -100,6 +101,17 @@ func (arguments Arguments) Unpack(v interface{}, data []byte) error { return arguments.unpackAtomic(v, marshalledValues) } +// Unpack2 performs the operation hexdata -> Go format. +func (arguments Arguments) Unpack2(data []byte) ([]interface{}, error) { + if len(data) == 0 { + if len(arguments.NonIndexed()) != 0 { + return nil, errors.New("abi: attempting to unmarshall an empty string while arguments are expected") + } + return make([]interface{}, 0), nil + } + return arguments.UnpackValues(data) +} + func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interface{}) error { var ( diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 27978dd48..3790956a4 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -363,7 +363,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call XDPoSChain.Call // callContract implements common code between normal and pending contract calls. // state is modified during execution, make sure to copy it if necessary. -func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.CallMsg, block *types.Block, statedb *state.StateDB) ([]byte, uint64, bool, error) { +func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.CallMsg, block *types.Block, statedb *state.StateDB) (ret []byte, usedGas uint64, failed bool, err error) { // Ensure message is initialized properly. if call.GasPrice == nil { call.GasPrice = big.NewInt(1) @@ -391,7 +391,8 @@ func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.Cal vmenv := vm.NewEVM(evmContext, statedb, nil, b.config, vm.Config{}) gaspool := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - return core.NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) + ret, usedGas, failed, err, _ = core.NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) + return } // SendTransaction updates the pending block to include the given transaction. diff --git a/core/state_processor.go b/core/state_processor.go index 07550658b..28248ba06 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -409,7 +409,7 @@ func ApplyTransaction(config *params.ChainConfig, tokensFee map[common.Address]* // End Bypass blacklist address // Apply the transaction to the current state (included in the env) - _, gas, failed, err := ApplyMessage(vmenv, msg, gp, coinbaseOwner) + _, gas, failed, err, _ := ApplyMessage(vmenv, msg, gp, coinbaseOwner) if err != nil { return nil, 0, err, false diff --git a/core/state_transition.go b/core/state_transition.go index 8a1387c3a..2d3d09780 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -131,7 +131,7 @@ func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool) *StateTransition // the gas used (which includes gas refunds) and an error if it failed. An error always // indicates a core error meaning that the message would always fail for that particular // state and would never be accepted within a block. -func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, owner common.Address) ([]byte, uint64, bool, error) { +func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, owner common.Address) ([]byte, uint64, bool, error, error) { return NewStateTransition(evm, msg, gp).TransitionDb(owner) } @@ -217,7 +217,7 @@ func (st *StateTransition) preCheck() error { // TransitionDb will transition the state by applying the current message and // returning the result including the the used gas. It returns an error if it // failed. An error indicates a consensus issue. -func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedGas uint64, failed bool, err error) { +func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedGas uint64, failed bool, err error, vmErr error) { if err = st.preCheck(); err != nil { return } @@ -230,10 +230,10 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG // Pay intrinsic gas gas, err := IntrinsicGas(st.data, contractCreation, homestead) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } if err = st.useGas(gas); err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } var ( @@ -263,7 +263,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG // sufficient balance to make the transfer happen. The first // balance transfer may never fail. if vmerr == vm.ErrInsufficientBalance { - return nil, 0, false, vmerr + return nil, 0, false, vmerr, nil } } st.refundGas() @@ -276,7 +276,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(new(big.Int).SetUint64(st.gasUsed()), st.gasPrice)) } - return ret, st.gasUsed(), vmerr != nil, err + return ret, st.gasUsed(), vmerr != nil, err, vmerr } func (st *StateTransition) refundGas() { diff --git a/core/token_validator.go b/core/token_validator.go index 7a5ac2b81..a1efae02f 100644 --- a/core/token_validator.go +++ b/core/token_validator.go @@ -112,7 +112,7 @@ func CallContractWithState(call ethereum.CallMsg, chain consensus.ChainContext, vmenv := vm.NewEVM(evmContext, statedb, nil, chain.Config(), vm.Config{}) gaspool := new(GasPool).AddGas(1000000) owner := common.Address{} - rval, _, _, err := NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) + rval, _, _, err, _ := NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner) if err != nil { return nil, err } diff --git a/eth/api_tracer.go b/eth/api_tracer.go index 87c9308fd..2cacf45b8 100644 --- a/eth/api_tracer.go +++ b/eth/api_tracer.go @@ -480,7 +480,7 @@ func (api *PrivateDebugAPI) traceBlock(ctx context.Context, block *types.Block, vmenv := vm.NewEVM(vmctx, statedb, XDCxState, api.config, vm.Config{}) owner := common.Address{} - if _, _, _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), owner); err != nil { + if _, _, _, err, _ := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), owner); err != nil { failed = err break } @@ -636,7 +636,7 @@ func (api *PrivateDebugAPI) traceTx(ctx context.Context, message core.Message, v vmenv := vm.NewEVM(vmctx, statedb, nil, api.config, vm.Config{Debug: true, Tracer: tracer}) owner := common.Address{} - ret, gas, failed, err := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas()), owner) + ret, gas, failed, err, _ := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas()), owner) if err != nil { return nil, fmt.Errorf("tracing failed: %v", err) } diff --git a/eth/tracers/tracers_test.go b/eth/tracers/tracers_test.go index e106d5a23..0be41b8cb 100644 --- a/eth/tracers/tracers_test.go +++ b/eth/tracers/tracers_test.go @@ -183,7 +183,7 @@ func TestPrestateTracerCreate2(t *testing.T) { t.Fatalf("failed to prepare transaction for tracing: %v", err) } st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas())) - if _, _, _, err = st.TransitionDb(common.Address{}); err != nil { + if _, _, _, err, _ = st.TransitionDb(common.Address{}); err != nil { t.Fatalf("failed to execute transaction: %v", err) } // Retrieve the trace result and compare against the etalon @@ -258,7 +258,7 @@ func TestCallTracer(t *testing.T) { t.Fatalf("failed to prepare transaction for tracing: %v", err) } st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas())) - if _, _, _, err = st.TransitionDb(common.Address{}); err != nil { + if _, _, _, err, _ = st.TransitionDb(common.Address{}); err != nil { t.Fatalf("failed to execute transaction: %v", err) } // Retrieve the trace result and compare against the etalon diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 9daf5875f..abceb9eee 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -32,6 +32,7 @@ import ( "github.com/XinFinOrg/XDC-Subnet/accounts" "github.com/XinFinOrg/XDC-Subnet/accounts/abi/bind" + "github.com/XinFinOrg/XDC-Subnet/accounts/abi" "github.com/XinFinOrg/XDC-Subnet/accounts/keystore" "github.com/XinFinOrg/XDC-Subnet/common" "github.com/XinFinOrg/XDC-Subnet/common/hexutil" @@ -1045,12 +1046,12 @@ type CallArgs struct { Data hexutil.Bytes `json:"data"` } -func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration) ([]byte, uint64, bool, error) { +func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration) ([]byte, uint64, bool, error, error) { defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now()) statedb, header, err := s.b.StateAndHeaderByNumber(ctx, blockNr) if statedb == nil || err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } // Set sender address or use a default if none specified addr := args.From @@ -1088,20 +1089,20 @@ func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr block, err := s.b.BlockByNumber(ctx, blockNr) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } author, err := s.b.GetEngine().Author(block.Header()) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } XDCxState, err := s.b.XDCxService().GetTradingState(block, author) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } // Get a new instance of the EVM. evm, vmError, err := s.b.GetEVM(ctx, msg, statedb, XDCxState, header, vmCfg) if err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } // Wait for the context to be done and cancel the evm. Even if the // EVM has finished, cancelling may be done (repeatedly) @@ -1114,18 +1115,64 @@ func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr // and apply the message. gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - res, gas, failed, err := core.ApplyMessage(evm, msg, gp, owner) + res, gas, failed, err, vmErr := core.ApplyMessage(evm, msg, gp, owner) if err := vmError(); err != nil { - return nil, 0, false, err + return nil, 0, false, err, nil } - return res, gas, failed, err + + // If the timer caused an abort, return an appropriate error message + if evm.Cancelled() { + return nil, 0, false, fmt.Errorf("execution aborted (timeout = %v)", timeout), nil + } + if err != nil { + return res, 0, false, fmt.Errorf("err: %w (supplied gas %d)", err, msg.Gas()), nil + } + return res, gas, failed, err, vmErr +} + +func newRevertError(res []byte) *revertError { + reason, errUnpack := abi.UnpackRevert(res) + err := errors.New("execution reverted") + if errUnpack == nil { + err = fmt.Errorf("execution reverted: %v", reason) + } + return &revertError{ + error: err, + reason: hexutil.Encode(res), + } +} + +// revertError is an API error that encompasses an EVM revertal with JSON error +// code and a binary data blob. +type revertError struct { + error + reason string // revert reason hex encoded +} + +// ErrorCode returns the JSON error code for a revertal. +// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +func (e *revertError) ErrorCode() int { + return 3 +} + +// ErrorData returns the hex encoded revert reason. +func (e *revertError) ErrorData() interface{} { + return e.reason } // Call executes the given transaction on the state for the given block number. // It doesn't make and changes in the state/blockchain and is useful to execute and retrieve values. func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber) (hexutil.Bytes, error) { - result, _, _, err := s.doCall(ctx, args, blockNr, vm.Config{}, 5*time.Second) - return (hexutil.Bytes)(result), err + result, _, failed, err, vmErr := s.doCall(ctx, args, blockNr, vm.Config{}, 5*time.Second) + if err != nil { + return nil, err + } + // If the result contains a revert reason, try to unpack and return it. + if failed && len(result) > 0 { + return nil, newRevertError(result) + } + + return (hexutil.Bytes)(result), vmErr } // EstimateGas returns an estimate of the amount of gas needed to execute the @@ -1150,29 +1197,58 @@ func (s *PublicBlockChainAPI) EstimateGas(ctx context.Context, args CallArgs) (h cap = hi // Create a helper to check if a gas allowance results in an executable transaction - executable := func(gas uint64) bool { + executable := func(gas uint64) (bool, []byte, error, error) { args.Gas = hexutil.Uint64(gas) - _, _, failed, err := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0) - if err != nil || failed { - log.Warn("[EstimateGas] api", "err", err) - return false + res, _, failed, err, vmErr := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0) + if err != nil { + if errors.Is(err, core.ErrIntrinsicGas) { + return false, nil, nil, nil // Special case, raise gas limit + } + return false, nil, err, nil // Bail out } - return true + if failed { + return false, res, nil, vmErr + } + + return true, nil, nil, nil } // Execute the binary search and hone in on an executable gas limit for lo+1 < hi { mid := (hi + lo) / 2 - if !executable(mid) { + ok, _, err, _ := executable(mid) + + // If the error is not nil(consensus error), it means the provided message + // call or transaction will never be accepted no matter how much gas it is + // assigned. Return the error directly, don't struggle any more. + if err != nil { + return 0, err + } + + if !ok { lo = mid } else { hi = mid } } + // Reject the transaction as invalid if it still fails at the highest allowance if hi == cap { - if !executable(hi) { - return 0, fmt.Errorf("gas required exceeds allowance or always failing transaction") + ok, res, err, vmErr := executable(hi) + if err != nil { + return 0, err + } + + if !ok { + if vmErr != vm.ErrOutOfGas { + if len(res) > 0 { + return 0, newRevertError(res) + } + return 0, vmErr + } + + // Otherwise, the specified gas cap is too low + return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) } } return hexutil.Uint64(hi), nil diff --git a/les/odr_test.go b/les/odr_test.go index 4161bcce0..0dd5b4a33 100644 --- a/les/odr_test.go +++ b/les/odr_test.go @@ -141,7 +141,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai //vmenv := core.NewEnv(statedb, config, bc, msg, header, vm.Config{}) gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) + ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) res = append(res, ret...) } } else { @@ -158,7 +158,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai vmenv := vm.NewEVM(context, statedb, nil, config, vm.Config{}) gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) + ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) if statedb.Error() == nil { res = append(res, ret...) } diff --git a/light/odr_test.go b/light/odr_test.go index 2ef2ea176..057e7cb49 100644 --- a/light/odr_test.go +++ b/light/odr_test.go @@ -188,7 +188,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, bc *core.BlockChain vmenv := vm.NewEVM(context, st, nil, config, vm.Config{}) gp := new(core.GasPool).AddGas(math.MaxUint64) owner := common.Address{} - ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) + ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner) res = append(res, ret...) if st.Error() != nil { return res, st.Error() diff --git a/tests/state_test_util.go b/tests/state_test_util.go index 02c87a03f..c1dfe6fa7 100644 --- a/tests/state_test_util.go +++ b/tests/state_test_util.go @@ -144,7 +144,7 @@ func (t *StateTest) Run(subtest StateSubtest, vmconfig vm.Config) (*state.StateD snapshot := statedb.Snapshot() coinbase := &t.json.Env.Coinbase - if _, _, _, err := core.ApplyMessage(evm, msg, gaspool, *coinbase); err != nil { + if _, _, _, err, _ := core.ApplyMessage(evm, msg, gaspool, *coinbase); err != nil { statedb.RevertToSnapshot(snapshot) } if logs := rlpHash(statedb.Logs()); logs != common.Hash(post.Logs) {