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

Adjust semantic tests to work on both EOF and legacy #15768

Merged
merged 2 commits into from
Jan 29, 2025
Merged

Adjust semantic tests to work on both EOF and legacy #15768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9851010
Remove obsolete semantic tests for the old-style .value() syntax
cameel Jan 24, 2025
3b1d78d
Adjust semantic tests where possible to make them run unchanged on bo…
cameel Jan 23, 2025
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
4 changes: 2 additions & 2 deletions test/libsolidity/semanticTests/builtinFunctions/keccak256_packed_complex_types.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ contract C {
x[2] = y[2] = uint120(type(uint).max - 3);
hash1 = keccak256(abi.encodePacked(x));
hash2 = keccak256(abi.encodePacked(y));
hash3 = keccak256(abi.encodePacked(this.f));
hash3 = keccak256(abi.encodePacked(C(address(0x1234))));
}
}
// ----
// f() -> 0xba4f20407251e4607cd66b90bfea19ec6971699c03e4a4f3ea737d5818ac27ae, 0xba4f20407251e4607cd66b90bfea19ec6971699c03e4a4f3ea737d5818ac27ae, 0x0e9229fb1d2cd02cee4b6c9f25497777014a8766e3479666d1c619066d2887ec
// f() -> 0xba4f20407251e4607cd66b90bfea19ec6971699c03e4a4f3ea737d5818ac27ae, 0xba4f20407251e4607cd66b90bfea19ec6971699c03e4a4f3ea737d5818ac27ae, 0xe7490fade3a8e31113ecb6c0d2635e28a6f5ca8359a57afe914827f41ddf0848
12 changes: 6 additions & 6 deletions test/libsolidity/semanticTests/constructor/no_callvalue_check.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ contract B3 { constructor() payable {} }
contract C {
function f() public payable returns (bool) {
// Make sure none of these revert.
new B1{value: 10}();
new B2{value: 10}();
new B3{value: 10}();
new B1{value: 10, salt: hex"00"}();
new B2{value: 10, salt: hex"01"}();
new B3{value: 10, salt: hex"02"}();
Comment on lines -12 to +14
Copy link
Member Author

Choose a reason for hiding this comment

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

Cases like this fail on EOF, because the default salt is 0 and therefore unsalted new always generates the same address. And deploying to the same address causes a revert unless the initcode is identical.

return true;
}
}
// ----
// f(), 2000 ether -> true
// gas irOptimized: 117623
// gas irOptimized: 117688
// gas irOptimized code: 1800
// gas legacy: 117821
// gas legacy: 117889
// gas legacy code: 4800
// gas legacyOptimized: 117690
// gas legacyOptimized: 117761
// gas legacyOptimized code: 4800
8 changes: 4 additions & 4 deletions test/libsolidity/semanticTests/errors/errors_by_parameter_type.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ contract C {
require(false, E3(S(1, true, "string literal")));
}
function e() external view {
require(false, E4(address(this)));
require(false, E4(address(0x1234)));
}
function f() external view {
require(false, E5(this.a));
require(false, E5(C(address(0x1234)).a));
}
}

Expand All @@ -41,5 +41,5 @@ contract C {
// b() -> FAILURE, hex"47e26897", hex"0000000000000000000000000000000000000000000000000000000000000001"
// c() -> FAILURE, hex"8f372c34", hex"0000000000000000000000000000000000000000000000000000000000000020", hex"000000000000000000000000000000000000000000000000000000000000000e", hex"737472696e67206c69746572616c000000000000000000000000000000000000"
// d() -> FAILURE, hex"5717173e", hex"0000000000000000000000000000000000000000000000000000000000000020", hex"0000000000000000000000000000000000000000000000000000000000000001", hex"0000000000000000000000000000000000000000000000000000000000000001", hex"0000000000000000000000000000000000000000000000000000000000000060", hex"000000000000000000000000000000000000000000000000000000000000000e", hex"737472696e67206c69746572616c000000000000000000000000000000000000"
// e() -> FAILURE, hex"7efef9ea", hex"000000000000000000000000c06afe3a8444fc0004668591e8306bfb9968e79e"
// f() -> FAILURE, hex"0c3f12eb", hex"c06afe3a8444fc0004668591e8306bfb9968e79e0dbe671f0000000000000000"
// e() -> FAILURE, hex"7efef9ea", hex"0000000000000000000000000000000000000000000000000000000000001234"
// f() -> FAILURE, hex"0c3f12eb", hex"00000000000000000000000000000000000012340dbe671f0000000000000000"
4 changes: 2 additions & 2 deletions test/libsolidity/semanticTests/errors/require_error_function_pointer_parameter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ contract C
function f() external view
{
// more than one stack slot
require(false, CustomError(this.e));
require(false, CustomError(C(address(0x1234)).e));
}
}

// ----
// f() -> FAILURE, hex"271b1dfa", hex"c06afe3a8444fc0004668591e8306bfb9968e79ef37cdc8e0000000000000000"
// f() -> FAILURE, hex"271b1dfa", hex"0000000000000000000000000000000000001234f37cdc8e0000000000000000"
4 changes: 2 additions & 2 deletions test/libsolidity/semanticTests/events/event_indexed_function.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
contract C {
event Test(function() external indexed);
function f() public {
emit Test(this.f);
emit Test(C(address(0x1234)).f);
}
}
// ----
// f() ->
// ~ emit Test(function): #0xc06afe3a8444fc0004668591e8306bfb9968e79e26121ff00000000000000000
// ~ emit Test(function): #0x123426121ff00000000000000000
8 changes: 4 additions & 4 deletions test/libsolidity/semanticTests/events/event_indexed_function2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ contract C {
event TestA(function() external indexed);
event TestB(function(uint256) external indexed);
function f1() public {
emit TestA(this.f1);
emit TestA(C(address(0x1234)).f1);
}
function f2(uint256 a) public {
emit TestB(this.f2);
emit TestB(C(address(0x1234)).f2);
}
}
// ----
// f1() ->
// ~ emit TestA(function): #0xc06afe3a8444fc0004668591e8306bfb9968e79ec27fc3050000000000000000
// ~ emit TestA(function): #0x1234c27fc3050000000000000000
// f2(uint256): 1 ->
// ~ emit TestB(function): #0xc06afe3a8444fc0004668591e8306bfb9968e79ebf3724af0000000000000000
// ~ emit TestB(function): #0x1234bf3724af0000000000000000
52 changes: 25 additions & 27 deletions test/libsolidity/semanticTests/externalContracts/snark.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pragma abicoder v2;

library Pairing {
struct G1Point {
uint X;
Expand Down Expand Up @@ -35,34 +37,26 @@ library Pairing {

/// @return r the sum of two points of G1
function add(G1Point memory p1, G1Point memory p2) internal returns (G1Point memory r) {
uint[4] memory input;
uint[6] memory input;
input[0] = p1.X;
input[1] = p1.Y;
input[2] = p2.X;
input[3] = p2.Y;
bool success;
assembly {
success := call(sub(gas(), 2000), 6, 0, input, 0xc0, r, 0x60)
// Use "invalid" to make gas estimation work
switch success case 0 { invalid() }
}
(bool success, bytes memory encodedResult) = address(6).call(abi.encode(input));
Comment on lines -38 to +45
Copy link
Member Author

Choose a reason for hiding this comment

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

This is something I don't really understand. According to https://www.evm.codes/precompiled ecadd (precompile at address 0x06) accepts 4 uints. However the original code was passing in 6 of them (0xc0 bytes). It reverts if I try to pass in 4.

Similarly, ecmul below seems to expect 4 rather than 3 parameters.

Copy link
Member Author

@cameel cameel Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, the reason I'm removing the assembly blocks in this file is of course that call() is only available on legacy and the equivalent extcall() only on EOF. We currently can't have assembly that works on both, but we can use <address>.call() in Solidity, which gets automatically translated to the right opcode.

This contract seems to be using assembly only to work around some problem related to gas estimation (and maybe also to make the call cheaper), which is not an issue in our tests.

Copy link
Member Author

@cameel cameel Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For the record, @ekpyron got to the bottom of it:

My guess would be that it's a typo in ancient versions of this (there's very old snippets of this piece of code around) (i.e. it should always have been 0x80 as size, but the oldest versions dating back to 2017 or older just had 0xC0, and since that also works as per spec, nobody ever fixed it, and instead the hard-coded cases in our implementation were adjusted to accommodate those - funny actually :-)) (see Zokrates/ZoKrates#1364 and such - and Zokrates/ZoKrates#1364 (comment) in particular :-))

#15788 fixes this.

require(success);
r = abi.decode(encodedResult, (G1Point));
}

/// @return r the product of a point on G1 and a scalar, i.e.
/// p == p.mul(1) and p.add(p) == p.mul(2) for all points p.
function mul(G1Point memory p, uint s) internal returns (G1Point memory r) {
uint[3] memory input;
uint[4] memory input;
input[0] = p.X;
input[1] = p.Y;
input[2] = s;
bool success;
assembly {
success := call(sub(gas(), 2000), 7, 0, input, 0x80, r, 0x60)
// Use "invalid" to make gas estimation work
switch success case 0 { invalid() }
}
(bool success, bytes memory encodedResult) = address(7).call(abi.encode(input));
require(success);
r = abi.decode(encodedResult, (G1Point));
}

/// @return the result of computing the pairing check
Expand All @@ -83,15 +77,19 @@ library Pairing {
input[i * 6 + 4] = p2[i].Y[0];
input[i * 6 + 5] = p2[i].Y[1];
}
uint[1] memory out;
bool success;
assembly {
success := call(sub(gas(), 2000), 8, 0, add(input, 0x20), mul(inputSize, 0x20), out, 0x20)
// Use "invalid" to make gas estimation work
switch success case 0 { invalid() }

bytes memory encodedInput = new bytes(inputSize * 32);
for (uint i = 0; i < inputSize; i++)
{
uint offset = (i + 1) * 32;
uint item = input[i];
assembly ("memory-safe") {
mstore(add(encodedInput, offset), item)
}
}
(bool success, bytes memory encodedResult) = address(8).call(encodedInput);
require(success);
return out[0] != 0;
return abi.decode(encodedResult, (bool));
}
function pairingProd2(G1Point memory a1, G2Point memory a2, G1Point memory b1, G2Point memory b2) internal returns (bool) {
G1Point[] memory p1 = new G1Point[](2);
Expand Down Expand Up @@ -294,11 +292,11 @@ contract Test {
// f() -> true
// g() -> true
// pair() -> true
// gas irOptimized: 270409
// gas legacy: 275219
// gas legacyOptimized: 266862
// gas irOptimized: 275319
// gas legacy: 293854
// gas legacyOptimized: 276409
// verifyTx() -> true
// ~ emit Verified(string): 0x20, 0x16, "Successfully verified."
// gas irOptimized: 785720
// gas legacy: 801903
// gas legacyOptimized: 770941
// gas irOptimized: 821446
// gas legacy: 914211
// gas legacyOptimized: 820319
32 changes: 16 additions & 16 deletions test/libsolidity/semanticTests/functionCall/failed_create.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,35 @@ contract D { constructor() payable {} }
contract C {
uint public x;
constructor() payable {}
function f(uint amount) public returns (D) {
function f(uint amount) public {
x++;
return (new D){value: amount}();
(new D){value: amount, salt: bytes32(x)}();
}
function stack(uint depth) public payable returns (address) {
function stack(uint depth) public payable {
if (depth > 0)
return this.stack(depth - 1);
this.stack(depth - 1);
else
return address(f(0));
f(0);
}
}
// ====
// EVMVersion: >=byzantium
// ----
// constructor(), 20 wei
// gas irOptimized: 61548
// gas irOptimized code: 104600
// gas legacy: 70147
// gas legacy code: 215400
// gas legacyOptimized: 61715
// gas legacyOptimized code: 106800
// f(uint256): 20 -> 0x137aa4dfc0911524504fcd4d98501f179bc13b4a
// gas irOptimized: 59688
// gas irOptimized code: 81800
// gas legacy: 64468
// gas legacy code: 145400
// gas legacyOptimized: 60443
// gas legacyOptimized code: 91200
// f(uint256): 20 ->
// x() -> 1
// f(uint256): 20 -> FAILURE
// x() -> 1
// stack(uint256): 1023 -> FAILURE
// gas irOptimized: 252410
// gas legacy: 476845
// gas legacyOptimized: 299061
// gas irOptimized: 298110
// gas legacy: 527207
// gas legacyOptimized: 353607
// x() -> 1
// stack(uint256): 10 -> 0x87948bd7ebbe13a00bfd930c93e4828ab18e3908
// stack(uint256): 10 ->
// x() -> 2
2 changes: 0 additions & 2 deletions test/libsolidity/semanticTests/functionCall/gas_and_value_basic.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ contract helper {
return flag;
}
}


contract test {
helper h;

Expand Down
48 changes: 0 additions & 48 deletions test/libsolidity/semanticTests/functionCall/gas_and_value_brace_syntax.sol
View file
Open in desktop

This file was deleted.

10 changes: 5 additions & 5 deletions test/libsolidity/semanticTests/functionTypes/address_member.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
contract C {
function f() public view returns (address a1, address a2) {
a1 = this.f.address;
this.f.address;
[this.f.address][0];
a2 = [this.f.address][0];
a1 = C(address(0x1234)).f.address;
C(address(0x1234)).f.address;
[C(address(0x1234)).f.address][0];
a2 = [C(address(0x1234)).f.address][0];
}
}
// ----
// f() -> 0xc06afe3a8444fc0004668591e8306bfb9968e79e, 0xc06afe3a8444fc0004668591e8306bfb9968e79e
// f() -> 0x1234, 0x1234
38 changes: 18 additions & 20 deletions test/libsolidity/semanticTests/functionTypes/function_external_delete_storage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
contract C {
function() external public x;
uint public y = 0;

function increment() public {
++y;
}
function f() public {}

function set() external {
x = this.increment;
x = this.f;
}

function isF() external returns (bool) {
return x == this.f;
}

function incrementIndirectly() public {
x();
function isZero() external returns (bool) {
function() external zero;
return x == zero;
}

function deleteFunction() public {
Expand All @@ -20,18 +22,14 @@ contract C {
}
}
// ----
// x() -> 0
// y() -> 0
// increment() ->
// y() -> 1
// isF() -> false
// isZero() -> true
// deleteFunction() ->
// isF() -> false
// isZero() -> true
// set() ->
// x() -> 0xc06afe3a8444fc0004668591e8306bfb9968e79ed09de08a0000000000000000
// increment() ->
// y() -> 2
// incrementIndirectly() ->
// y() -> 3
// isF() -> true
// isZero() -> false
// deleteFunction() ->
// increment() ->
// y() -> 4
// incrementIndirectly() -> FAILURE
// y() -> 4
// isF() -> false
// isZero() -> true
12 changes: 6 additions & 6 deletions test/libsolidity/semanticTests/immutable/multi_creation.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,20 @@ contract C {
uint public y;
constructor() {
a = 3;
x = (new A()).f();
y = (new B()).f();
x = (new A{salt: hex"00"}()).f();
y = (new B{salt: hex"00"}()).f();
}
function f() public returns (uint256, uint, uint) {
return (a, (new A()).f(), (new B()).f());
return (a, (new A{salt: hex"01"}()).f(), (new B{salt: hex"01"}()).f());
}
}
// ----
// f() -> 3, 7, 5
// gas irOptimized: 86796
// gas irOptimized: 86892
// gas irOptimized code: 37200
// gas legacy: 87727
// gas legacy: 87839
// gas legacy code: 60800
// gas legacyOptimized: 86770
// gas legacyOptimized: 86870
// gas legacyOptimized code: 37200
// x() -> 7
// y() -> 5
Loading