Skip to content

Commit be6f734

Browse files
authored
Use the EVM call gas syscall variants (#436)
- Emit the `call_evm` and `delegate_call_evm` syscalls for contract calls. - The call gas is no longer ignored. --------- Signed-off-by: Cyrill Leutwiler <[email protected]>
1 parent 25ee4ee commit be6f734

File tree

9 files changed

+87
-204
lines changed

9 files changed

+87
-204
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ Supported `polkadot-sdk` rev: `unstable2507`
1616
- Standard JSON mode: Don't forward EVM bytecode related output selections to solc.
1717
- The supported `polkadot-sdk` release is `unstable2507`.
1818
- The `INVALID` opcode and OOB memory accesses now consume all remaining gas.
19+
- Emit the `call_evm` and `delegate_call_evm` syscalls for contract calls.
1920

2021
### Fixed:
2122
- The missing `STOP` instruction at the end of `code` blocks.
2223
- The missing bounds check in the internal sbrk implementation.
24+
- The call gas is no longer ignored.
2325

2426
## v0.5.0
2527

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
pragma solidity ^0.8;
4+
5+
// Use a non-zero call gas that works with call gas clipping but not with a truncate.
6+
7+
/* runner.json
8+
{
9+
"differential": true,
10+
"actions": [
11+
{
12+
"Upload": {
13+
"code": {
14+
"Solidity": {
15+
"contract": "Other"
16+
}
17+
}
18+
}
19+
},
20+
{
21+
"Instantiate": {
22+
"code": {
23+
"Solidity": {
24+
"contract": "CallGas"
25+
}
26+
},
27+
"data": "1000000000000000000000000000000000000000000000000000000000000001"
28+
}
29+
}
30+
]
31+
}
32+
*/
33+
34+
contract Other {
35+
address public last;
36+
uint public foo;
37+
38+
fallback() external {
39+
last = msg.sender;
40+
foo += 1;
41+
}
42+
}
43+
44+
contract CallGas {
45+
constructor(uint _gas) payable {
46+
Other other = new Other();
47+
address(other).call{ gas: _gas }(hex"");
48+
assert(other.last() == address(this));
49+
}
50+
}

crates/integration/contracts/Send.sol

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,6 @@ pragma solidity ^0.8;
2323
"data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a"
2424
}
2525
},
26-
{
27-
"VerifyCall": {
28-
"success": true
29-
}
30-
},
3126
{
3227
"Call": {
3328
"dest": {
@@ -36,23 +31,13 @@ pragma solidity ^0.8;
3631
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000001"
3732
}
3833
},
39-
{
40-
"VerifyCall": {
41-
"success": false
42-
}
43-
},
4434
{
4535
"Call": {
4636
"dest": {
4737
"Instantiated": 0
4838
},
4939
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000000"
5040
}
51-
},
52-
{
53-
"VerifyCall": {
54-
"success": false
55-
}
5641
}
5742
]
5843
}

crates/integration/contracts/Transfer.sol

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,6 @@ pragma solidity ^0.8;
2323
"data": "1c8d16b30000000000000000000000000303030303030303030303030303030303030303000000000000000000000000000000000000000000000000000000000000000a"
2424
}
2525
},
26-
{
27-
"VerifyCall": {
28-
"success": true
29-
}
30-
},
3126
{
3227
"Call": {
3328
"dest": {
@@ -36,23 +31,13 @@ pragma solidity ^0.8;
3631
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000001"
3732
}
3833
},
39-
{
40-
"VerifyCall": {
41-
"success": false
42-
}
43-
},
4434
{
4535
"Call": {
4636
"dest": {
4737
"Instantiated": 0
4838
},
4939
"data": "fb9e8d050000000000000000000000000000000000000000000000000000000000000000"
5040
}
51-
},
52-
{
53-
"VerifyCall": {
54-
"success": false
55-
}
5641
}
5742
]
5843
}

crates/integration/src/tests.rs

Lines changed: 1 addition & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ test_spec!(add_mod_mul_mod, "AddModMulModTester", "AddModMulMod.sol");
6666
test_spec!(memory_bounds, "MemoryBounds", "MemoryBounds.sol");
6767
test_spec!(selfdestruct, "Selfdestruct", "Selfdestruct.sol");
6868
test_spec!(clz, "CountLeadingZeros", "CountLeadingZeros.sol");
69+
test_spec!(call_gas, "CallGas", "CallGas.sol");
6970

7071
fn instantiate(path: &str, contract: &str) -> Vec<SpecsAction> {
7172
vec![Instantiate {
@@ -451,50 +452,6 @@ fn ext_code_size() {
451452
.run();
452453
}
453454

454-
#[test]
455-
#[should_panic(expected = "ReentranceDenied")]
456-
fn send_denies_reentrancy() {
457-
let value = 1000;
458-
Specs {
459-
actions: vec![
460-
instantiate("contracts/Send.sol", "Send").remove(0),
461-
Call {
462-
origin: TestAddress::Alice,
463-
dest: TestAddress::Instantiated(0),
464-
value,
465-
gas_limit: None,
466-
storage_deposit_limit: None,
467-
data: Contract::send_self(U256::from(value)).calldata,
468-
},
469-
],
470-
differential: false,
471-
..Default::default()
472-
}
473-
.run();
474-
}
475-
476-
#[test]
477-
#[should_panic(expected = "ReentranceDenied")]
478-
fn transfer_denies_reentrancy() {
479-
let value = 1000;
480-
Specs {
481-
actions: vec![
482-
instantiate("contracts/Transfer.sol", "Transfer").remove(0),
483-
Call {
484-
origin: TestAddress::Alice,
485-
dest: TestAddress::Instantiated(0),
486-
value,
487-
gas_limit: None,
488-
storage_deposit_limit: None,
489-
data: Contract::transfer_self(U256::from(value)).calldata,
490-
},
491-
],
492-
differential: false,
493-
..Default::default()
494-
}
495-
.run();
496-
}
497-
498455
#[test]
499456
fn create2_salt() {
500457
let salt = U256::from(777);

crates/llvm-context/src/polkavm/evm/call.rs

Lines changed: 27 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@ use inkwell::values::BasicValue;
44

55
use crate::polkavm::context::Context;
66

7-
const STATIC_CALL_FLAG: u32 = 0b0001_0000;
8-
const REENTRANT_CALL_FLAG: u32 = 0b0000_1000;
9-
const SOLIDITY_TRANSFER_GAS_STIPEND_THRESHOLD: u64 = 2300;
7+
const STATIC_CALL_FLAG: u64 = 0b0001_0000;
8+
const REENTRANT_CALL_FLAG: u64 = 0b0000_1000;
109

1110
/// Translates a contract call.
1211
pub fn call<'ctx>(
@@ -38,33 +37,12 @@ pub fn call<'ctx>(
3837
let output_length_pointer = context.build_alloca_at_entry(context.xlen_type(), "output_length");
3938
context.build_store(output_length_pointer, output_length)?;
4039

41-
let (flags, deposit_limit_value) = if static_call {
42-
let flags = REENTRANT_CALL_FLAG | STATIC_CALL_FLAG;
43-
(
44-
context.xlen_type().const_int(flags as u64, false),
45-
context.word_type().const_zero(),
46-
)
40+
let flags = if static_call {
41+
REENTRANT_CALL_FLAG | STATIC_CALL_FLAG
4742
} else {
48-
call_reentrancy_heuristic(context, gas, input_length, output_length)?
43+
REENTRANT_CALL_FLAG
4944
};
5045

51-
let deposit_pointer = context.build_alloca_at_entry(context.word_type(), "deposit_pointer");
52-
context.build_store(deposit_pointer, deposit_limit_value)?;
53-
54-
let flags_and_callee = revive_runtime_api::calling_convention::pack_hi_lo_reg(
55-
context.builder(),
56-
context.llvm(),
57-
flags,
58-
address_pointer.to_int(context),
59-
"address_and_callee",
60-
)?;
61-
let deposit_and_value = revive_runtime_api::calling_convention::pack_hi_lo_reg(
62-
context.builder(),
63-
context.llvm(),
64-
deposit_pointer.to_int(context),
65-
value_pointer.to_int(context),
66-
"deposit_and_value",
67-
)?;
6846
let input_data = revive_runtime_api::calling_convention::pack_hi_lo_reg(
6947
context.builder(),
7048
context.llvm(),
@@ -85,10 +63,10 @@ pub fn call<'ctx>(
8563
.build_runtime_call(
8664
name,
8765
&[
88-
flags_and_callee.into(),
89-
context.register_type().const_all_ones().into(),
90-
context.register_type().const_all_ones().into(),
91-
deposit_and_value.into(),
66+
context.xlen_type().const_int(flags, false).into(),
67+
address_pointer.to_int(context).into(),
68+
value_pointer.to_int(context).into(),
69+
clip_call_gas(context, gas)?,
9270
input_data.into(),
9371
output_data.into(),
9472
],
@@ -111,7 +89,7 @@ pub fn call<'ctx>(
11189

11290
pub fn delegate_call<'ctx>(
11391
context: &mut Context<'ctx>,
114-
_gas: inkwell::values::IntValue<'ctx>,
92+
gas: inkwell::values::IntValue<'ctx>,
11593
address: inkwell::values::IntValue<'ctx>,
11694
input_offset: inkwell::values::IntValue<'ctx>,
11795
input_length: inkwell::values::IntValue<'ctx>,
@@ -132,18 +110,6 @@ pub fn delegate_call<'ctx>(
132110
let output_length_pointer = context.build_alloca_at_entry(context.xlen_type(), "output_length");
133111
context.build_store(output_length_pointer, output_length)?;
134112

135-
let deposit_pointer = context.build_alloca_at_entry(context.word_type(), "deposit_pointer");
136-
context.build_store(deposit_pointer, context.word_type().const_all_ones())?;
137-
138-
let flags = context.xlen_type().const_int(0u64, false);
139-
140-
let flags_and_callee = revive_runtime_api::calling_convention::pack_hi_lo_reg(
141-
context.builder(),
142-
context.llvm(),
143-
flags,
144-
address_pointer.to_int(context),
145-
"address_and_callee",
146-
)?;
147113
let input_data = revive_runtime_api::calling_convention::pack_hi_lo_reg(
148114
context.builder(),
149115
context.llvm(),
@@ -164,10 +130,9 @@ pub fn delegate_call<'ctx>(
164130
.build_runtime_call(
165131
name,
166132
&[
167-
flags_and_callee.into(),
168-
context.register_type().const_all_ones().into(),
169-
context.register_type().const_all_ones().into(),
170-
deposit_pointer.to_int(context).into(),
133+
context.xlen_type().const_int(0u64, false).into(),
134+
address_pointer.to_int(context).into(),
135+
clip_call_gas(context, gas)?,
171136
input_data.into(),
172137
output_data.into(),
173138
],
@@ -201,82 +166,22 @@ pub fn linker_symbol<'ctx>(
201166
context.build_load_address(context.get_global(path)?.into())
202167
}
203168

204-
/// The Solidity `address.transfer` and `address.send` call detection heuristic.
205-
///
206-
/// # Why
207-
/// This heuristic is an additional security feature to guard against re-entrancy attacks
208-
/// in case contract authors violate Solidity best practices and use `address.transfer` or
209-
/// `address.send`.
210-
/// While contract authors are supposed to never use `address.transfer` or `address.send`,
211-
/// for a small cost we can be extra defensive about it.
212-
///
213-
/// # How
214-
/// The gas stipend emitted by solc for `transfer` and `send` is not static, thus:
215-
/// - Dynamically allow re-entrancy only for calls considered not transfer or send.
216-
/// - Detected balance transfers will supply 0 deposit limit instead of `u256::MAX`.
217-
///
218-
/// Calls are considered transfer or send if:
219-
/// - (Input length | Output lenght) == 0;
220-
/// - Gas <= 2300;
221-
///
222-
/// # Returns
223-
/// The call flags xlen `IntValue` and the deposit limit word `IntValue`.
224-
fn call_reentrancy_heuristic<'ctx>(
225-
context: &mut Context<'ctx>,
169+
/// The runtime implements gas as `u64` so we clip the stipend to `u64::MAX`.
170+
fn clip_call_gas<'ctx>(
171+
context: &Context<'ctx>,
226172
gas: inkwell::values::IntValue<'ctx>,
227-
input_length: inkwell::values::IntValue<'ctx>,
228-
output_length: inkwell::values::IntValue<'ctx>,
229-
) -> anyhow::Result<(
230-
inkwell::values::IntValue<'ctx>,
231-
inkwell::values::IntValue<'ctx>,
232-
)> {
233-
// Branch-free SSA implementation: First derive the heuristic boolean (int1) value.
234-
let input_length_or_output_length =
235-
context
236-
.builder()
237-
.build_or(input_length, output_length, "input_length_or_output_length")?;
238-
let is_no_input_no_output = context.builder().build_int_compare(
239-
inkwell::IntPredicate::EQ,
240-
context.xlen_type().const_zero(),
241-
input_length_or_output_length,
242-
"is_no_input_no_output",
243-
)?;
244-
let gas_stipend = context
245-
.word_type()
246-
.const_int(SOLIDITY_TRANSFER_GAS_STIPEND_THRESHOLD, false);
247-
let is_gas_stipend_for_transfer_or_send = context.builder().build_int_compare(
248-
inkwell::IntPredicate::ULE,
249-
gas,
250-
gas_stipend,
251-
"is_gas_stipend_for_transfer_or_send",
252-
)?;
253-
let is_balance_transfer = context.builder().build_and(
254-
is_no_input_no_output,
255-
is_gas_stipend_for_transfer_or_send,
256-
"is_balance_transfer",
257-
)?;
258-
let is_regular_call = context
259-
.builder()
260-
.build_not(is_balance_transfer, "is_balance_transfer_inverted")?;
261-
262-
// Call flag: Left shift the heuristic boolean value.
263-
let is_regular_call_xlen = context.builder().build_int_z_extend(
264-
is_regular_call,
265-
context.xlen_type(),
266-
"is_balance_transfer_xlen",
267-
)?;
268-
let call_flags = context.builder().build_left_shift(
269-
is_regular_call_xlen,
270-
context.xlen_type().const_int(3, false),
271-
"flags",
272-
)?;
173+
) -> anyhow::Result<inkwell::values::BasicValueEnum<'ctx>> {
174+
let builder = context.builder();
273175

274-
// Deposit limit value: Sign-extended the heuristic boolean value.
275-
let deposit_limit_value = context.builder().build_int_s_extend(
276-
is_regular_call,
277-
context.word_type(),
278-
"deposit_limit_value",
176+
let clipped = context.register_type().const_all_ones();
177+
let is_overflow = builder.build_int_compare(
178+
inkwell::IntPredicate::UGT,
179+
gas,
180+
builder.build_int_z_extend(clipped, context.word_type(), "gas_clipped")?,
181+
"is_gas_overflow",
279182
)?;
183+
let truncated = builder.build_int_truncate(gas, context.register_type(), "gas_truncated")?;
184+
let call_gas = builder.build_select(is_overflow, clipped, truncated, "call_gas")?;
280185

281-
Ok((call_flags, deposit_limit_value))
186+
Ok(call_gas)
282187
}

0 commit comments

Comments
 (0)