Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[evm] manually correct gas refund in case opcode execution returns Er… #3690

Merged
merged 15 commits into from
Dec 2, 2022
Merged

[evm] manually correct gas refund in case opcode execution returns Er… #3690

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4bce536
[evm] manually correct gas refund in case opcode execution returns Er…
dustinxie Sep 27, 2022
19e66ce
Merge branch 'master' into testing
millken Nov 23, 2022
bb3c901
fix gas refund, update tests
millken Nov 23, 2022
c9eb62b
fix gas refund, update tests
millken Nov 23, 2022
d2cb675
fix gas refund on testnet 12117999
millken Nov 25, 2022
6b40bfe
Merge branch 'master' into testing
millken Nov 29, 2022
b1aaf5c
Merge branch 'master' into testing
millken Nov 29, 2022
323cf28
update testdata
millken Nov 29, 2022
b6d04fc
fix tests
millken Nov 29, 2022
f696353
Merge branch 'master' into testing
millken Nov 30, 2022
adc64f7
Merge branch 'master' into testing
millken Dec 1, 2022
86b5d52
fix comment in json
millken Dec 1, 2022
baf3c27
add more test cases
millken Dec 2, 2022
c5a64f6
add more test cases
millken Dec 2, 2022
0352587
Merge branch 'master' into testing
millken Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions action/protocol/execution/evm/evm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func prepareStateDB(ctx context.Context, sm protocol.StateManager) (*StateDBAdap
opts = append(opts, NotCheckPutStateErrorOption())
}
if !featureCtx.CorrectGasRefund {
opts = append(opts, NotCorrectGasRefundOption())
opts = append(opts, ManualCorrectGasRefundOption())
}

return NewStateDBAdapter(
Expand Down Expand Up @@ -422,17 +422,18 @@ func executeInEVM(ctx context.Context, evmParams *Params, stateDB *StateDBAdapte
// After EIP-3529: refunds are capped to gasUsed / 5
refund = (evmParams.gas - remainingGas) / params.RefundQuotientEIP3529
}
// adjust refund due to dynamicGas
// before London EVM activation (at Okhotsk height), in certain cases dynamicGas
// has caused gas refund to change, which needs to be manually adjusted after
// the tx is reverted. After Okhotsk height, it is fixed inside RevertToSnapshot()
var (
refundLastSnapshot = stateDB.RefundAtLastSnapshot()
currentRefund = stateDB.GetRefund()
featureCtx = protocol.MustGetFeatureCtx(ctx)
deltaRefundByDynamicGas = evm.DeltaRefundByDynamicGas
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

txcontext is not an ideal place to store the deltarefund. StateDB should be the only place to interact with evm from outside. we may add an interface to set delta refund value. Consequently, we can all stateDB.GetRefundWithDelta, and do not to fetch feature context here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the interface (SetDelta, GetRefundWithDelta) needs to be implemented in go-ethereum and iotex-core, but not necessary for go-ethereum. the current deltaRefund is just a temporary variable, I think there is no problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TxContext.Origin and TxContext.GasPrice are also used in our iotex-core code, so i think it's fine to use it

featureCtx = protocol.MustGetFeatureCtx(ctx)
)
if evmErr != nil && !featureCtx.CorrectGasRefund && refundLastSnapshot > 0 && refundLastSnapshot != currentRefund {
if refundLastSnapshot > currentRefund {
stateDB.AddRefund(refundLastSnapshot - currentRefund)
if evmErr != nil && !featureCtx.CorrectGasRefund && deltaRefundByDynamicGas != 0 {
if deltaRefundByDynamicGas > 0 {
stateDB.SubRefund(uint64(deltaRefundByDynamicGas))
} else {
stateDB.SubRefund(currentRefund - refundLastSnapshot)
stateDB.AddRefund(uint64(-deltaRefundByDynamicGas))
}
}
if refund > stateDB.GetRefund() {
Expand Down
23 changes: 8 additions & 15 deletions action/protocol/execution/evm/evmstatedbadapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ type (
blockHeight uint64
executionHash hash.Hash256
refund uint64
refundAtLastSnapshot uint64
refundSnapshot map[int]uint64
cachedContract contractMap
contractSnapshot map[int]contractMap // snapshots of contracts
Expand All @@ -76,7 +75,7 @@ type (
fixSnapshotOrder bool
revertLog bool
notCheckPutStateError bool
notCorrectGasRefund bool
manualCorrectGasRefund bool
}
)

Expand Down Expand Up @@ -147,10 +146,13 @@ func NotCheckPutStateErrorOption() StateDBAdapterOption {
}
}

// NotCorrectGasRefundOption set correctGasRefund as true
func NotCorrectGasRefundOption() StateDBAdapterOption {
// ManualCorrectGasRefundOption set manualCorrectGasRefund as true
func ManualCorrectGasRefundOption() StateDBAdapterOption {
return func(adapter *StateDBAdapter) error {
adapter.notCorrectGasRefund = true
// before London EVM activation (at Okhotsk height), in certain cases dynamicGas
// has caused gas refund to change, which needs to be manually adjusted after
// the tx is reverted. After Okhotsk height, it is fixed inside RevertToSnapshot()
adapter.manualCorrectGasRefund = true
return nil
}
}
Expand Down Expand Up @@ -553,10 +555,7 @@ func (stateDB *StateDBAdapter) RevertToSnapshot(snapshot int) {
return
}
// restore gas refund
if stateDB.notCorrectGasRefund {
// check if refund has changed from last snapshot (due to dynamicGas)
stateDB.refundAtLastSnapshot = stateDB.refundSnapshot[snapshot]
} else {
if !stateDB.manualCorrectGasRefund {
stateDB.refund = stateDB.refundSnapshot[snapshot]
}
// restore logs and txLogs
Expand Down Expand Up @@ -641,11 +640,6 @@ func (stateDB *StateDBAdapter) RevertToSnapshot(snapshot int) {
}
}

// RefundAtLastSnapshot returns refund at last snapshot
func (stateDB *StateDBAdapter) RefundAtLastSnapshot() uint64 {
return stateDB.refundAtLastSnapshot
}

func (stateDB *StateDBAdapter) cachedContractAddrs() []hash.Hash160 {
addrs := make([]hash.Hash160, 0, len(stateDB.cachedContract))
for addr := range stateDB.cachedContract {
Expand Down Expand Up @@ -1036,7 +1030,6 @@ func (stateDB *StateDBAdapter) getNewContract(addr hash.Hash160) (Contract, erro

// clear clears local changes
func (stateDB *StateDBAdapter) clear() {
stateDB.refundAtLastSnapshot = 0
stateDB.refundSnapshot = make(map[int]uint64)
stateDB.cachedContract = make(contractMap)
stateDB.contractSnapshot = make(map[int]contractMap)
Expand Down
27 changes: 27 additions & 0 deletions action/protocol/execution/protocol_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,33 @@ func TestIstanbulEVM(t *testing.T) {
t.Run("CVE-2021-39137-attack-replay", func(t *testing.T) {
NewSmartContractTest(t, "testdata/CVE-2021-39137-attack-replay.json")
})
t.Run("err-write-protection", func(t *testing.T) {
NewSmartContractTest(t, "testdata-istanbul/write-protection.json")
})
t.Run("err-write-protection-twice-delta-0", func(t *testing.T) {
// hit errWriteProtection 2 times, delta is 0
NewSmartContractTest(t, "testdata-istanbul/write-protection-001.json")
})
t.Run("err-write-protection-once-delta-0", func(t *testing.T) {
// hit errWriteProtection 1 times, delta is 0
NewSmartContractTest(t, "testdata-istanbul/write-protection-002.json")
})
t.Run("err-write-protection-twice-delta-0-0", func(t *testing.T) {
// hit errWriteProtection twice, delta is not 0
NewSmartContractTest(t, "testdata-istanbul/write-protection-003.json")
})
t.Run("err-write-protection-twice-delta-0-1", func(t *testing.T) {
// hit errWriteProtection twice, first delta is not 0, second delta is 0
NewSmartContractTest(t, "testdata-istanbul/write-protection-004.json")
})
t.Run("err-write-protection-once-delta-1", func(t *testing.T) {
// hit errWriteProtection once, delta is not 0
NewSmartContractTest(t, "testdata-istanbul/write-protection-005.json")
})
}

func TestSmartContractTest(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can move to a t.Run() like those above?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a debug code, has been removed

NewSmartContractTest(t, "testdata-istanbul/write-protection-005.json")
}

func TestLondonEVM(t *testing.T) {
Expand Down
47 changes: 47 additions & 0 deletions action/protocol/execution/testdata-istanbul/write-protection-001.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"initGenesis": {
"isBering" : true,
"isIceland" : true
},
"initBalances": [{
"account": "io1mflp9m6hcgm2qcghchsdqj3z3eccrnekx9p0ms",
"rawBalance": "1000000000000000000000000000"
}],
"deployments": [{
"rawByteCode": "608060405234801561001057600080fd5b50610800806100206000396000f3fe608060405234801561001057600080fd5b50600436106100625760003560e01c8063167517ce146100675780633fa4f245146100b55780634e70b1dc146100d357806367e404ce146100f1578063c6dad08214610125578063d1e0f3081461012f575b600080fd5b6100b36004803603604081101561007d57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610191565b005b6100bd610346565b6040518082815260200191505060405180910390f35b6100db61034c565b6040518082815260200191505060405180910390f35b6100f9610352565b604051808273ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b61012d610378565b005b61017b6004803603604081101561014557600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610431565b6040518082815260200191505060405180910390f35b60008114156101b3576004600060018152602001908152602001600020600090555b600060608373ffffffffffffffffffffffffffffffffffffffff1683604051602401808281526020019150506040516020818303038152906040527f6466414b000000000000000000000000000000000000000000000000000000007bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506040518082805190602001908083835b6020831061028f578051825260208201915060208101905060208303925061026c565b6001836020036101000a038019825116818451168082178552505050505050905001915050600060405180830381855afa9150503d80600081146102ef576040519150601f19603f3d011682016040523d82523d6000602084013e6102f4565b606091505b50915091507f3b0a8ddef325df2bfdfa6b430ae4c8421841cd135bfa8fb5e432f200787520bb8260405180821515815260200191505060405180910390a1600083141561034057600080fd5b50505050565b60025481565b60005481565b600160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b604051610384906105ae565b604051809103906000f0801580156103a0573d6000803e3d6000fd5b50600360006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff160217905550600160046000600181526020019081526020016000208190555060026004600060028152602001908152602001600020819055506003600460006003815260200190815260200160002081905550565b60008073ffffffffffffffffffffffffffffffffffffffff168373ffffffffffffffffffffffffffffffffffffffff16141561048d57600360009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1692505b3073ffffffffffffffffffffffffffffffffffffffff1663167517ce8460006040518363ffffffff1660e01b8152600401808373ffffffffffffffffffffffffffffffffffffffff16815260200182815260200192505050600060405180830381600087803b1580156104ff57600080fd5b505af1925050508015610510575060015b6105a3573073ffffffffffffffffffffffffffffffffffffffff1663167517ce8460016040518363ffffffff1660e01b8152600401808373ffffffffffffffffffffffffffffffffffffffff16815260200182815260200192505050600060405180830381600087803b15801561058657600080fd5b505af115801561059a573d6000803e3d6000fd5b505050506105a4565b5b6001905092915050565b61020f806105bc8339019056fe608060405234801561001057600080fd5b506101ef806100206000396000f3fe60806040526004361061003f5760003560e01c80633fa4f245146100445780634e70b1dc1461006f5780636466414b1461009a57806367e404ce146100c8575b600080fd5b34801561005057600080fd5b50610059610109565b6040518082815260200191505060405180910390f35b34801561007b57600080fd5b5061008461010f565b6040518082815260200191505060405180910390f35b6100c6600480360360208110156100b057600080fd5b8101908080359060200190929190505050610115565b005b3480156100d457600080fd5b506100dd610193565b604051808273ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b60025481565b60005481565b8060008190555033600160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff160217905550346002819055507f9f9fb434574749b74458e0ddc3cf5fd5bdb1b009c8615e825606b53724576f3560405160405180910390a150565b600160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff168156fea2646970667358221220dc7b79728e1a021337a246ea4100bea60a986cd86fb67c31ebc34b868a7c5b4064736f6c634300060c0033a264697066735822122014ab730ff529680b3b79508b582c7f724ec58dce75085f480e9dd74864f849d664736f6c634300060c0033",
"rawPrivateKey": "cfa6ef757dee2e50351620dca002d32b9c090cfda55fb81f37f1d26b273743f1",
"rawAmount": "0",
"rawGasLimit": 5000000,
"rawGasPrice": "0",
"rawExpectedGasConsumed": 628043,
"expectedStatus": 1,
"expectedBalances": [],
"comment": "deploy write protection contract"
}],
"executions": [{
"rawPrivateKey": "cfa6ef757dee2e50351620dca002d32b9c090cfda55fb81f37f1d26b273743f1",
"rawByteCode": "c6dad082",
"rawAmount": "0",
"rawGasLimit": 1000000,
"rawGasPrice": "0",
"rawAccessList": [],
"rawExpectedGasConsumed": 223105,
"expectedStatus": 1,
"hasReturnValue": true,
"rawReturnValue": "",
"comment": "call make"
}, {
"rawPrivateKey": "cfa6ef757dee2e50351620dca002d32b9c090cfda55fb81f37f1d26b273743f1",
"rawByteCode": "d1e0f30800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000084",
"rawAmount": "0",
"rawGasLimit": 6000000,
"rawGasPrice": "0",
"rawAccessList": [],
"rawExpectedGasConsumed": 5980648,
"expectedStatus": 1,
"expectedLogs": [{},{}],
"hasReturnValue": true,
"rawReturnValue": "",
"comment": "call make"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong comment

}]
}
59 changes: 59 additions & 0 deletions action/protocol/execution/testdata-istanbul/write-protection-001.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.6.6;

// NOTE: Deploy this contract first
contract B {
// NOTE: storage layout must be the same as contract A
uint public num;
address public sender;
uint public value;
event Done();

function setVars(uint _num) public payable {
num = _num;
sender = msg.sender;
value = msg.value;
emit Done();
}
}

contract A {
uint public num;
address public sender;
uint public value;
address private c;
mapping(uint => uint) private _a;
event Success(bool);

function make() public {
c = address(new B());
_a[1] = 1;
_a[2] = 2;
_a[3] = 3;
}

function callStatic(address _contract,uint _num) public {
if (_num == 0) {
delete _a[1];
}
(bool success, bytes memory _) = _contract.staticcall(
abi.encodeWithSignature("setVars(uint256)", _num)
);
emit Success(success);
if (_num == 0) {
revert();
}
}

function setVars(address _contract, uint _num) public returns (uint256) {
if (_contract == address(0)) {
_contract = c;
}
try this.callStatic(_contract,0) {
} catch {
this.callStatic(_contract,1);
}

return 1;
}
}
47 changes: 47 additions & 0 deletions action/protocol/execution/testdata-istanbul/write-protection-002.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"initGenesis": {
"isBering" : true,
"isIceland" : true
},
"initBalances": [{
"account": "io1mflp9m6hcgm2qcghchsdqj3z3eccrnekx9p0ms",
"rawBalance": "1000000000000000000000000000"
}],
"deployments": [{
"rawByteCode": "608060405234801561001057600080fd5b50610674806100206000396000f3fe608060405234801561001057600080fd5b50600436106100575760003560e01c80633fa4f2451461005c5780634e70b1dc1461007a57806367e404ce14610098578063c6dad082146100cc578063d1e0f308146100d6575b600080fd5b610064610138565b6040518082815260200191505060405180910390f35b61008261013e565b6040518082815260200191505060405180910390f35b6100a0610144565b604051808273ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b6100d461016a565b005b610122600480360360408110156100ec57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610223565b6040518082815260200191505060405180910390f35b60025481565b60005481565b600160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b60405161017690610422565b604051809103906000f080158015610192573d6000803e3d6000fd5b50600360006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff160217905550600160046000600181526020019081526020016000208190555060026004600060028152602001908152602001600020819055506003600460006003815260200190815260200160002081905550565b60008073ffffffffffffffffffffffffffffffffffffffff168373ffffffffffffffffffffffffffffffffffffffff16141561027f57600360009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1692505b600460006001815260200190815260200160002060009055600060608473ffffffffffffffffffffffffffffffffffffffff1684604051602401808281526020019150506040516020818303038152906040527f6466414b000000000000000000000000000000000000000000000000000000007bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506040518082805190602001908083835b602083106103735780518252602082019150602081019050602083039250610350565b6001836020036101000a038019825116818451168082178552505050505050905001915050600060405180830381855afa9150503d80600081146103d3576040519150601f19603f3d011682016040523d82523d6000602084013e6103d8565b606091505b50915091507f3b0a8ddef325df2bfdfa6b430ae4c8421841cd135bfa8fb5e432f200787520bb8260405180821515815260200191505060405180910390a160019250505092915050565b61020f806104308339019056fe608060405234801561001057600080fd5b506101ef806100206000396000f3fe60806040526004361061003f5760003560e01c80633fa4f245146100445780634e70b1dc1461006f5780636466414b1461009a57806367e404ce146100c8575b600080fd5b34801561005057600080fd5b50610059610109565b6040518082815260200191505060405180910390f35b34801561007b57600080fd5b5061008461010f565b6040518082815260200191505060405180910390f35b6100c6600480360360208110156100b057600080fd5b8101908080359060200190929190505050610115565b005b3480156100d457600080fd5b506100dd610193565b604051808273ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b60025481565b60005481565b8060008190555033600160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff160217905550346002819055507f9f9fb434574749b74458e0ddc3cf5fd5bdb1b009c8615e825606b53724576f3560405160405180910390a150565b600160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff168156fea2646970667358221220767a39a75f6558c8cd67a6a7d5f3c563c41b8b525a636bd0bf208f54cbb050d464736f6c634300060c0033a2646970667358221220d8c4080ef0ef989fb44d95667c95127f088ee79c32c18be82d378ea8fe4fbc7c64736f6c634300060c0033",
"rawPrivateKey": "cfa6ef757dee2e50351620dca002d32b9c090cfda55fb81f37f1d26b273743f1",
"rawAmount": "0",
"rawGasLimit": 5000000,
"rawGasPrice": "0",
"rawExpectedGasConsumed": 509168,
"expectedStatus": 1,
"expectedBalances": [],
"comment": "deploy write protection contract"
}],
"executions": [{
"rawPrivateKey": "cfa6ef757dee2e50351620dca002d32b9c090cfda55fb81f37f1d26b273743f1",
"rawByteCode": "c6dad082",
"rawAmount": "0",
"rawGasLimit": 1000000,
"rawGasPrice": "0",
"rawAccessList": [],
"rawExpectedGasConsumed": 223083,
"expectedStatus": 1,
"hasReturnValue": true,
"rawReturnValue": "",
"comment": "call make"
}, {
"rawPrivateKey": "cfa6ef757dee2e50351620dca002d32b9c090cfda55fb81f37f1d26b273743f1",
"rawByteCode": "d1e0f30800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000084",
"rawAmount": "0",
"rawGasLimit": 6000000,
"rawGasPrice": "0",
"rawAccessList": [],
"rawExpectedGasConsumed": 5892833,
"expectedStatus": 1,
"expectedLogs": [{}],
"hasReturnValue": true,
"rawReturnValue": "",
"comment": "call make"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong comment

}]
}
47 changes: 47 additions & 0 deletions action/protocol/execution/testdata-istanbul/write-protection-002.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.6.6;

// NOTE: Deploy this contract first
contract B {
// NOTE: storage layout must be the same as contract A
uint public num;
address public sender;
uint public value;
event Done();

function setVars(uint _num) public payable {
num = _num;
sender = msg.sender;
value = msg.value;
emit Done();
}
}

contract A {
uint public num;
address public sender;
uint public value;
address private c;
mapping(uint => uint) private _a;
event Success(bool);

function make() public {
c = address(new B());
_a[1] = 1;
_a[2] = 2;
_a[3] = 3;
}

function setVars(address _contract, uint _num) public returns (uint256) {
if (_contract == address(0)) {
_contract = c;
}
delete _a[1];
(bool success, bytes memory _) = _contract.staticcall(
abi.encodeWithSignature("setVars(uint256)", _num)
);
emit Success(success);

return 1;
}
}
Loading