Skip to content

Commit 4e79e18

Browse files
andrei-marinicaandrei-marinica
authored
Merge pull request #2289 from multiversx/small-int-conv-workaround
mBufferFromSmallIntSigned workaround + Rust VM fix
2 parents 5087df0 + c4d4db4 commit 4e79e18

File tree

15 files changed

+661
-8
lines changed

15 files changed

+661
-8
lines changed

‎chain/vm/src/host/vm_hooks/vh_handler/vh_managed_types.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ mod vh_managed_buffer;
44
mod vh_managed_map;
55

66
use multiversx_chain_vm_executor::VMHooksEarlyExit;
7+
use num_traits::Signed;
78

89
use crate::{
910
host::{
@@ -110,7 +111,7 @@ impl<C: VMHooksContext> VMHooksHandler<C> {
110111
buffer_handle: RawHandle,
111112
) -> Result<i64, VMHooksEarlyExit> {
112113
let bytes = self.context.m_types_lock().mb_to_bytes(buffer_handle);
113-
let bi = num_bigint::BigInt::from_bytes_be(num_bigint::Sign::Plus, &bytes);
114+
let bi = num_bigint::BigInt::from_signed_bytes_be(&bytes);
114115
if let Some(small) = big_int_to_i64(&bi) {
115116
Ok(small)
116117
} else {
@@ -129,13 +130,19 @@ impl<C: VMHooksContext> VMHooksHandler<C> {
129130
Ok(())
130131
}
131132

133+
/// This method has a bug that converts negative numbers to their absolute values.
134+
///
135+
/// The bug will be kept here, until it is also fixed on mainnet, to allow consistent testing.
136+
///
137+
/// The framework avoids this VM hook, starting with v0.64.2.
132138
pub fn mb_from_small_int_signed(
133139
&self,
134140
buffer_handle: RawHandle,
135141
value: i64,
136142
) -> Result<(), VMHooksEarlyExit> {
137143
let bi = num_bigint::BigInt::from(value);
138-
let bytes = big_int_signed_bytes(&bi);
144+
// TODO: remove `.abs()` once the bug is fixed
145+
let bytes = big_int_signed_bytes(&bi.abs());
139146
self.context.m_types_lock().mb_set(buffer_handle, bytes);
140147
Ok(())
141148
}

‎contracts/feature-tests/basic-features/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ publish = false
88
[lib]
99
path = "src/basic_features_main.rs"
1010

11+
[features]
12+
small-int-bug = ["multiversx-sc/small-int-bug"]
13+
1114
[dependencies.multiversx-sc]
1215
version = "0.64.1"
1316
path = "../../../framework/base"

‎contracts/feature-tests/basic-features/sc-config.toml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ add-unlabelled = false
88
add-endpoints = ["init", "load_bytes", "store_bytes"]
99
kill_legacy_callback = true
1010

11+
# can be used for testing the buggy mBufferFromSmallIntSigned VM hook
12+
# TODO: remove once it's fixed
13+
[contracts.basic-features-small-int-bug]
14+
add-unlabelled = false
15+
add-endpoints = ["init", "store_i64", "load_i64"]
16+
kill_legacy_callback = true
17+
features = ["small-int-bug"]
18+
ei = "ignore" # bypasses the deprecated VM hook check
19+
1120
[[proxy]]
1221
path = "src/basic_features_proxy.rs"
1322
add-unlabelled = false

‎contracts/feature-tests/basic-features/scenarios/storage_big_int.scen.json‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,67 @@
137137
"gas": "*",
138138
"refund": "*"
139139
}
140+
},
141+
{
142+
"step": "scCall",
143+
"id": "store-negative",
144+
"tx": {
145+
"from": "address:an_account",
146+
"to": "sc:basic-features",
147+
"function": "store_big_int",
148+
"arguments": [
149+
"-456"
150+
],
151+
"gasLimit": "50,000,000",
152+
"gasPrice": "0"
153+
},
154+
"expect": {
155+
"out": [],
156+
"status": "",
157+
"logs": "*",
158+
"gas": "*",
159+
"refund": "*"
160+
}
161+
},
162+
{
163+
"step": "checkState",
164+
"accounts": {
165+
"sc:basic-features": {
166+
"nonce": "0",
167+
"balance": "0",
168+
"storage": {
169+
"str:big_int": "-456"
170+
},
171+
"code": "mxsc:../output/basic-features.mxsc.json"
172+
},
173+
"address:an_account": {
174+
"nonce": "*",
175+
"balance": "*",
176+
"storage": {},
177+
"code": ""
178+
}
179+
}
180+
},
181+
{
182+
"step": "scCall",
183+
"id": "load-negative",
184+
"tx": {
185+
"from": "address:an_account",
186+
"to": "sc:basic-features",
187+
"function": "load_big_int",
188+
"arguments": [],
189+
"gasLimit": "50,000,000",
190+
"gasPrice": "0"
191+
},
192+
"expect": {
193+
"out": [
194+
"-456"
195+
],
196+
"status": "",
197+
"logs": "*",
198+
"gas": "*",
199+
"refund": "*"
200+
}
140201
}
141202
]
142203
}

‎contracts/feature-tests/basic-features/scenarios/storage_i64.scen.json‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,67 @@
137137
"gas": "*",
138138
"refund": "*"
139139
}
140+
},
141+
{
142+
"step": "scCall",
143+
"id": "store-negative",
144+
"tx": {
145+
"from": "address:an_account",
146+
"to": "sc:basic-features",
147+
"function": "store_i64",
148+
"arguments": [
149+
"-456"
150+
],
151+
"gasLimit": "50,000,000",
152+
"gasPrice": "0"
153+
},
154+
"expect": {
155+
"out": [],
156+
"status": "",
157+
"logs": "*",
158+
"gas": "*",
159+
"refund": "*"
160+
}
161+
},
162+
{
163+
"step": "checkState",
164+
"accounts": {
165+
"sc:basic-features": {
166+
"nonce": "0",
167+
"balance": "0",
168+
"storage": {
169+
"str:i64": "-456"
170+
},
171+
"code": "mxsc:../output/basic-features.mxsc.json"
172+
},
173+
"address:an_account": {
174+
"nonce": "*",
175+
"balance": "*",
176+
"storage": {},
177+
"code": ""
178+
}
179+
}
180+
},
181+
{
182+
"step": "scCall",
183+
"id": "load-negative",
184+
"tx": {
185+
"from": "address:an_account",
186+
"to": "sc:basic-features",
187+
"function": "load_i64",
188+
"arguments": [],
189+
"gasLimit": "50,000,000",
190+
"gasPrice": "0"
191+
},
192+
"expect": {
193+
"out": [
194+
"-456"
195+
],
196+
"status": "",
197+
"logs": "*",
198+
"gas": "*",
199+
"refund": "*"
200+
}
140201
}
141202
]
142203
}

‎contracts/feature-tests/basic-features/scenarios/storage_i64_bug.scen.json‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
{
2+
"name": "storage-small-int-bug",
3+
"steps": [
4+
{
5+
"step": "setState",
6+
"accounts": {
7+
"sc:basic-features": {
8+
"nonce": "0",
9+
"balance": "0",
10+
"code": "mxsc:../output/basic-features-small-int-bug.mxsc.json"
11+
},
12+
"address:an_account": {
13+
"nonce": "0",
14+
"balance": "0"
15+
}
16+
}
17+
},
18+
{
19+
"step": "scCall",
20+
"id": "store-negative-bug",
21+
"comment": "acknowledges a VM bug",
22+
"tx": {
23+
"from": "address:an_account",
24+
"to": "sc:basic-features",
25+
"function": "store_i64",
26+
"arguments": [
27+
"-456"
28+
],
29+
"gasLimit": "50,000,000",
30+
"gasPrice": "0"
31+
},
32+
"expect": {
33+
"out": [],
34+
"status": "",
35+
"logs": "*",
36+
"gas": "*",
37+
"refund": "*"
38+
}
39+
},
40+
{
41+
"step": "checkState",
42+
"accounts": {
43+
"sc:basic-features": {
44+
"nonce": "0",
45+
"balance": "0",
46+
"storage": {
47+
"str:i64": "456"
48+
},
49+
"code": "*"
50+
},
51+
"address:an_account": {
52+
"nonce": "*",
53+
"balance": "*",
54+
"storage": {},
55+
"code": ""
56+
}
57+
}
58+
}
59+
]
60+
}

‎contracts/feature-tests/basic-features/tests/basic_features_scenario_go_test.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,11 @@ fn storage_i_64_bad_go() {
421421
world().run("scenarios/storage_i64_bad.scen.json");
422422
}
423423

424+
#[test]
425+
fn storage_i_64_bug_go() {
426+
world().run("scenarios/storage_i64_bug.scen.json");
427+
}
428+
424429
#[test]
425430
fn storage_load_from_address_go() {
426431
world().run("scenarios/storage_load_from_address.scen.json");

‎contracts/feature-tests/basic-features/tests/basic_features_scenario_rs_test.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,12 @@ fn storage_i_64_bad_rs() {
444444
world().run("scenarios/storage_i64_bad.scen.json");
445445
}
446446

447+
#[test]
448+
#[ignore = "requires the small-int-bug feature to reproduce"]
449+
fn storage_i_64_bug_rs() {
450+
world().run("scenarios/storage_i64_bug.scen.json");
451+
}
452+
447453
#[test]
448454
fn storage_load_from_address_rs() {
449455
world().run("scenarios/storage_load_from_address.scen.json");

0 commit comments

Comments
 (0)