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.

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

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

Merged
merged 15 commits into from
Dec 2, 2022
Merged
19 changes: 10 additions & 9 deletions action/protocol/execution/evm/evm.go
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 !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
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
23 changes: 23 additions & 0 deletions action/protocol/execution/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,29 @@ 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,but no revert
NewSmartContractTest(t, "testdata-istanbul/write-protection-005.json")
})
}

func TestLondonEVM(t *testing.T) {
Expand Down
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

}]
}
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;
}
}
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

}]
}
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