Skip to content

Commit f5ff5a3

Browse files
webmaster128webmaster128
authored
Merge pull request #136 from confio/remove_set_gas
Make gas limit immutable in `cosmwasm_vm::instance::Instance`
2 parents d4f2bfe + 201829c commit f5ff5a3

File tree

5 files changed

+131
-49
lines changed

5 files changed

+131
-49
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
- Remove export `check_api_compatibility`. The VM will take care of calling it.
3131
- Let `check_api_compatibility` check imports by fully qualified identifier
3232
`<module>.<name>`.
33+
- Make gas limit immutable in `cosmwasm_vm::instance::Instance`. It is passed
34+
once at construction time and cannot publicly be manipulated anymore.
3335

3436
## 0.6
3537

‎lib/vm/src/backends/singlepass.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,10 @@ pub fn set_gas(instance: &mut Instance, limit: u64) {
4040

4141
pub fn get_gas(instance: &Instance) -> u64 {
4242
let used = metering::get_points_used(instance);
43-
GAS_LIMIT - used
43+
// when running out of gas, get_points_used can exceed GAS_LIMIT
44+
if used < GAS_LIMIT {
45+
GAS_LIMIT - used
46+
} else {
47+
0
48+
}
4449
}

‎lib/vm/src/cache.rs

Lines changed: 92 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -88,28 +88,33 @@ where
8888
}
8989

9090
/// get instance returns a wasmer Instance tied to a previously saved wasm
91-
pub fn get_instance(&mut self, id: &[u8], deps: Extern<S, A>) -> Result<Instance<S, A>, Error> {
91+
pub fn get_instance(
92+
&mut self,
93+
id: &[u8],
94+
deps: Extern<S, A>,
95+
gas_limit: u64,
96+
) -> Result<Instance<S, A>, Error> {
9297
let hash = WasmHash::generate(&id);
9398

9499
// pop from lru cache if present
95100
if let Some(cache) = &mut self.instances {
96101
if let Some(cached_instance) = cache.pop(&hash) {
97102
self.stats.hits_instance += 1;
98-
return Ok(Instance::from_wasmer(cached_instance, deps));
103+
return Ok(Instance::from_wasmer(cached_instance, deps, gas_limit));
99104
}
100105
}
101106

102107
// try from the module cache
103108
let res = self.modules.load_with_backend(hash, backend());
104109
if let Ok(module) = res {
105110
self.stats.hits_module += 1;
106-
return Instance::from_module(&module, deps);
111+
return Instance::from_module(&module, deps, gas_limit);
107112
}
108113

109114
// fall back to wasm cache (and re-compiling) - this is for backends that don't support serialization
110115
let wasm = self.load_wasm(id)?;
111116
self.stats.misses += 1;
112-
Instance::from_code(&wasm, deps)
117+
Instance::from_code(&wasm, deps, gas_limit)
113118
}
114119

115120
pub fn store_instance(&mut self, id: &[u8], instance: Instance<S, A>) -> Option<Extern<S, A>> {
@@ -135,6 +140,7 @@ mod test {
135140
use cosmwasm::mock::{dependencies, mock_env, MockApi, MockStorage};
136141
use cosmwasm::types::coin;
137142

143+
static TESTING_GAS_LIMIT: u64 = 400_000;
138144
static CONTRACT_0_7: &[u8] = include_bytes!("../testdata/contract_0.7.wasm");
139145

140146
#[test]
@@ -170,7 +176,7 @@ mod test {
170176
let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() };
171177
let id = cache.save_wasm(CONTRACT_0_7).unwrap();
172178
let deps = dependencies(20);
173-
let _instance = cache.get_instance(&id, deps).unwrap();
179+
let _instance = cache.get_instance(&id, deps, TESTING_GAS_LIMIT).unwrap();
174180
assert_eq!(cache.stats.hits_instance, 0);
175181
assert_eq!(cache.stats.hits_module, 1);
176182
assert_eq!(cache.stats.misses, 0);
@@ -184,11 +190,11 @@ mod test {
184190
let deps1 = dependencies(20);
185191
let deps2 = dependencies(20);
186192
let deps3 = dependencies(20);
187-
let instance1 = cache.get_instance(&id, deps1).unwrap();
193+
let instance1 = cache.get_instance(&id, deps1, TESTING_GAS_LIMIT).unwrap();
188194
cache.store_instance(&id, instance1);
189-
let instance2 = cache.get_instance(&id, deps2).unwrap();
195+
let instance2 = cache.get_instance(&id, deps2, TESTING_GAS_LIMIT).unwrap();
190196
cache.store_instance(&id, instance2);
191-
let instance3 = cache.get_instance(&id, deps3).unwrap();
197+
let instance3 = cache.get_instance(&id, deps3, TESTING_GAS_LIMIT).unwrap();
192198
cache.store_instance(&id, instance3);
193199
assert_eq!(cache.stats.hits_instance, 2);
194200
assert_eq!(cache.stats.hits_module, 1);
@@ -201,7 +207,7 @@ mod test {
201207
let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() };
202208
let id = cache.save_wasm(CONTRACT_0_7).unwrap();
203209
let deps = dependencies(20);
204-
let mut instance = cache.get_instance(&id, deps).unwrap();
210+
let mut instance = cache.get_instance(&id, deps, TESTING_GAS_LIMIT).unwrap();
205211

206212
// run contract
207213
let env = mock_env(&instance.api, "creator", &coin("1000", "earth"), &[]);
@@ -219,7 +225,7 @@ mod test {
219225
let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() };
220226
let id = cache.save_wasm(CONTRACT_0_7).unwrap();
221227
let deps = dependencies(20);
222-
let mut instance = cache.get_instance(&id, deps).unwrap();
228+
let mut instance = cache.get_instance(&id, deps, TESTING_GAS_LIMIT).unwrap();
223229

224230
// init contract
225231
let env = mock_env(&instance.api, "creator", &coin("1000", "earth"), &[]);
@@ -252,7 +258,7 @@ mod test {
252258
let deps2 = dependencies(20);
253259

254260
// init instance 1
255-
let mut instance = cache.get_instance(&id, deps1).unwrap();
261+
let mut instance = cache.get_instance(&id, deps1, TESTING_GAS_LIMIT).unwrap();
256262
let env = mock_env(&instance.api, "owner1", &coin("1000", "earth"), &[]);
257263
let msg = r#"{"verifier": "sue", "beneficiary": "mary"}"#.as_bytes();
258264
let res = call_init(&mut instance, &env, msg).unwrap();
@@ -261,7 +267,7 @@ mod test {
261267
let deps1 = cache.store_instance(&id, instance).unwrap();
262268

263269
// init instance 2
264-
let mut instance = cache.get_instance(&id, deps2).unwrap();
270+
let mut instance = cache.get_instance(&id, deps2, TESTING_GAS_LIMIT).unwrap();
265271
let env = mock_env(&instance.api, "owner2", &coin("500", "earth"), &[]);
266272
let msg = r#"{"verifier": "bob", "beneficiary": "john"}"#.as_bytes();
267273
let res = call_init(&mut instance, &env, msg).unwrap();
@@ -270,7 +276,7 @@ mod test {
270276
let deps2 = cache.store_instance(&id, instance).unwrap();
271277

272278
// run contract 2 - just sanity check - results validate in contract unit tests
273-
let mut instance = cache.get_instance(&id, deps2).unwrap();
279+
let mut instance = cache.get_instance(&id, deps2, TESTING_GAS_LIMIT).unwrap();
274280
let env = mock_env(
275281
&instance.api,
276282
"bob",
@@ -284,7 +290,7 @@ mod test {
284290
let _ = cache.store_instance(&id, instance).unwrap();
285291

286292
// run contract 1 - just sanity check - results validate in contract unit tests
287-
let mut instance = cache.get_instance(&id, deps1).unwrap();
293+
let mut instance = cache.get_instance(&id, deps1, TESTING_GAS_LIMIT).unwrap();
288294
let env = mock_env(
289295
&instance.api,
290296
"sue",
@@ -297,4 +303,76 @@ mod test {
297303
assert_eq!(1, msgs.len());
298304
let _ = cache.store_instance(&id, instance);
299305
}
306+
307+
#[test]
308+
#[cfg(feature = "default-singlepass")]
309+
fn resets_gas_when_reusing_instance() {
310+
let tmp_dir = TempDir::new().unwrap();
311+
let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() };
312+
let id = cache.save_wasm(CONTRACT_0_7).unwrap();
313+
314+
let deps1 = dependencies(20);
315+
let deps2 = dependencies(20);
316+
317+
// Init from module cache
318+
let mut instance1 = cache.get_instance(&id, deps1, TESTING_GAS_LIMIT).unwrap();
319+
assert_eq!(cache.stats.hits_module, 1);
320+
assert_eq!(cache.stats.hits_instance, 0);
321+
assert_eq!(cache.stats.misses, 0);
322+
let original_gas = instance1.get_gas();
323+
324+
// Consume some gas
325+
let env = mock_env(&instance1.api, "owner1", &coin("1000", "earth"), &[]);
326+
let msg = r#"{"verifier": "sue", "beneficiary": "mary"}"#.as_bytes();
327+
call_init(&mut instance1, &env, msg).unwrap();
328+
assert!(instance1.get_gas() < original_gas);
329+
cache.store_instance(&id, instance1).unwrap();
330+
331+
// Init from instance cache
332+
let instance2 = cache.get_instance(&id, deps2, TESTING_GAS_LIMIT).unwrap();
333+
assert_eq!(cache.stats.hits_module, 1);
334+
assert_eq!(cache.stats.hits_instance, 1);
335+
assert_eq!(cache.stats.misses, 0);
336+
assert_eq!(instance2.get_gas(), TESTING_GAS_LIMIT);
337+
}
338+
339+
#[test]
340+
#[cfg(feature = "default-singlepass")]
341+
fn recovers_from_out_of_gas() {
342+
let tmp_dir = TempDir::new().unwrap();
343+
let mut cache = unsafe { CosmCache::new(tmp_dir.path(), 10).unwrap() };
344+
let id = cache.save_wasm(CONTRACT_0_7).unwrap();
345+
346+
let deps1 = dependencies(20);
347+
let deps2 = dependencies(20);
348+
349+
// Init from module cache
350+
let mut instance1 = cache.get_instance(&id, deps1, 10).unwrap();
351+
assert_eq!(cache.stats.hits_module, 1);
352+
assert_eq!(cache.stats.hits_instance, 0);
353+
assert_eq!(cache.stats.misses, 0);
354+
355+
// Consume some gas. This fails
356+
let env1 = mock_env(&instance1.api, "owner1", &coin("1000", "earth"), &[]);
357+
let msg1 = r#"{"verifier": "sue", "beneficiary": "mary"}"#.as_bytes();
358+
match call_init(&mut instance1, &env1, msg1) {
359+
Err(Error::RuntimeErr { .. }) => (), // all good, continue
360+
Err(e) => panic!("unexpected error, {:?}", e),
361+
Ok(_) => panic!("call_init must run out of gas"),
362+
}
363+
assert_eq!(instance1.get_gas(), 0);
364+
cache.store_instance(&id, instance1).unwrap();
365+
366+
// Init from instance cache
367+
let mut instance2 = cache.get_instance(&id, deps2, TESTING_GAS_LIMIT).unwrap();
368+
assert_eq!(cache.stats.hits_module, 1);
369+
assert_eq!(cache.stats.hits_instance, 1);
370+
assert_eq!(cache.stats.misses, 0);
371+
assert_eq!(instance2.get_gas(), TESTING_GAS_LIMIT);
372+
373+
// Now it works
374+
let env2 = mock_env(&instance2.api, "owner2", &coin("500", "earth"), &[]);
375+
let msg2 = r#"{"verifier": "bob", "beneficiary": "john"}"#.as_bytes();
376+
call_init(&mut instance2, &env2, msg2).unwrap();
377+
}
300378
}

‎lib/vm/src/instance.rs

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ where
3131
S: Storage + 'static,
3232
A: Api + 'static,
3333
{
34-
pub fn from_code(code: &[u8], deps: Extern<S, A>) -> Result<Self> {
34+
pub fn from_code(code: &[u8], deps: Extern<S, A>, gas_limit: u64) -> Result<Self> {
3535
let module = compile(code)?;
36-
Instance::from_module(&module, deps)
36+
Instance::from_module(&module, deps, gas_limit)
3737
}
3838

39-
pub fn from_module(module: &Module, deps: Extern<S, A>) -> Result<Self> {
39+
pub fn from_module(module: &Module, deps: Extern<S, A>, gas_limit: u64) -> Result<Self> {
4040
// copy this so it can be moved into the closures, without pulling in deps
4141
let api = deps.api;
4242
let import_obj = imports! {
@@ -72,10 +72,15 @@ where
7272
},
7373
};
7474
let wasmer_instance = module.instantiate(&import_obj).context(WasmerErr {})?;
75-
Ok(Instance::from_wasmer(wasmer_instance, deps))
75+
Ok(Instance::from_wasmer(wasmer_instance, deps, gas_limit))
7676
}
7777

78-
pub fn from_wasmer(wasmer_instance: wasmer_runtime_core::Instance, deps: Extern<S, A>) -> Self {
78+
pub fn from_wasmer(
79+
mut wasmer_instance: wasmer_runtime_core::Instance,
80+
deps: Extern<S, A>,
81+
gas_limit: u64,
82+
) -> Self {
83+
set_gas(&mut wasmer_instance, gas_limit);
7984
let res = Instance {
8085
wasmer_instance: wasmer_instance,
8186
api: deps.api,
@@ -95,10 +100,6 @@ where
95100
get_gas(&self.wasmer_instance)
96101
}
97102

98-
pub fn set_gas(&mut self, gas: u64) {
99-
set_gas(&mut self.wasmer_instance, gas)
100-
}
101-
102103
pub fn with_storage<F: FnMut(&mut S)>(&self, func: F) {
103104
with_storage_from_context(self.wasmer_instance.context(), func)
104105
}
@@ -145,32 +146,26 @@ where
145146
#[cfg(test)]
146147
mod test {
147148
use crate::calls::{call_handle, call_init, call_query};
148-
use crate::testing::mock_instance;
149+
use crate::testing::{mock_instance, mock_instance_with_gas_limit};
149150
use cosmwasm::mock::mock_env;
150151
use cosmwasm::types::coin;
151152

152153
static CONTRACT_0_7: &[u8] = include_bytes!("../testdata/contract_0.7.wasm");
153154

154155
#[test]
155156
#[cfg(feature = "default-cranelift")]
156-
fn get_and_set_gas_cranelift_noop() {
157-
let mut instance = mock_instance(&CONTRACT_0_7);
157+
fn set_get_and_gas_cranelift_noop() {
158+
let instance = mock_instance_with_gas_limit(&CONTRACT_0_7, 123321);
158159
let orig_gas = instance.get_gas();
159-
assert!(orig_gas > 1000);
160-
// this is a no-op
161-
instance.set_gas(123456);
162-
assert_eq!(orig_gas, instance.get_gas());
160+
assert_eq!(orig_gas, 1_000_000);
163161
}
164162

165163
#[test]
166164
#[cfg(feature = "default-singlepass")]
167-
fn get_and_set_gas_singlepass_works() {
168-
let mut instance = mock_instance(&CONTRACT_0_7);
165+
fn set_get_and_gas_singlepass_works() {
166+
let instance = mock_instance_with_gas_limit(&CONTRACT_0_7, 123321);
169167
let orig_gas = instance.get_gas();
170-
assert!(orig_gas > 1000000);
171-
// it is updated to whatever we set it with
172-
instance.set_gas(123456);
173-
assert_eq!(123456, instance.get_gas());
168+
assert_eq!(orig_gas, 123321);
174169
}
175170

176171
#[test]
@@ -185,8 +180,7 @@ mod test {
185180
#[cfg(feature = "default-singlepass")]
186181
fn contract_deducts_gas() {
187182
let mut instance = mock_instance(&CONTRACT_0_7);
188-
let orig_gas = 200_000;
189-
instance.set_gas(orig_gas);
183+
let orig_gas = instance.get_gas();
190184

191185
// init contract
192186
let env = mock_env(&instance.api, "creator", &coin("1000", "earth"), &[]);
@@ -200,7 +194,7 @@ mod test {
200194
assert_eq!(init_used, 52_468);
201195

202196
// run contract - just sanity check - results validate in contract unit tests
203-
instance.set_gas(orig_gas);
197+
let gas_before_handle = instance.get_gas();
204198
let env = mock_env(
205199
&instance.api,
206200
"verifies",
@@ -212,17 +206,15 @@ mod test {
212206
let msgs = res.unwrap().messages;
213207
assert_eq!(1, msgs.len());
214208

215-
let handle_used = orig_gas - instance.get_gas();
209+
let handle_used = gas_before_handle - instance.get_gas();
216210
println!("handle used: {}", handle_used);
217211
assert_eq!(handle_used, 86_749);
218212
}
219213

220214
#[test]
221215
#[cfg(feature = "default-singlepass")]
222216
fn contract_enforces_gas_limit() {
223-
let mut instance = mock_instance(&CONTRACT_0_7);
224-
let orig_gas = 20_000;
225-
instance.set_gas(orig_gas);
217+
let mut instance = mock_instance_with_gas_limit(&CONTRACT_0_7, 20_000);
226218

227219
// init contract
228220
let env = mock_env(&instance.api, "creator", &coin("1000", "earth"), &[]);
@@ -235,23 +227,21 @@ mod test {
235227
#[cfg(feature = "default-singlepass")]
236228
fn query_works_with_metering() {
237229
let mut instance = mock_instance(&CONTRACT_0_7);
238-
let orig_gas = 200_000;
239-
instance.set_gas(orig_gas);
240230

241231
// init contract
242232
let env = mock_env(&instance.api, "creator", &coin("1000", "earth"), &[]);
243233
let msg = r#"{"verifier": "verifies", "beneficiary": "benefits"}"#.as_bytes();
244234
let _res = call_init(&mut instance, &env, msg).unwrap().unwrap();
245235

246236
// run contract - just sanity check - results validate in contract unit tests
247-
instance.set_gas(orig_gas);
237+
let gas_before_query = instance.get_gas();
248238
// we need to encode the key in base64
249239
let msg = r#"{"verifier":{}}"#.as_bytes();
250240
let res = call_query(&mut instance, msg).unwrap();
251241
let answer = res.unwrap();
252242
assert_eq!(answer.as_slice(), "verifies".as_bytes());
253243

254-
let query_used = orig_gas - instance.get_gas();
244+
let query_used = gas_before_query - instance.get_gas();
255245
println!("query used: {}", query_used);
256246
assert_eq!(query_used, 44_919);
257247
}

‎lib/vm/src/testing.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,17 @@ use crate::calls::{call_handle, call_init, call_query};
1414
use crate::compatability::check_api_compatibility;
1515
use crate::instance::Instance;
1616

17+
/// Gas limit for testing
18+
static DEFAULT_GAS_LIMIT: u64 = 500_000;
19+
1720
pub fn mock_instance(wasm: &[u8]) -> Instance<MockStorage, MockApi> {
21+
mock_instance_with_gas_limit(wasm, DEFAULT_GAS_LIMIT)
22+
}
23+
24+
pub fn mock_instance_with_gas_limit(wasm: &[u8], gas_limit: u64) -> Instance<MockStorage, MockApi> {
1825
check_api_compatibility(wasm).unwrap();
1926
let deps = dependencies(20);
20-
Instance::from_code(wasm, deps).unwrap()
27+
Instance::from_code(wasm, deps, gas_limit).unwrap()
2128
}
2229

2330
// init mimicks the call signature of the smart contracts.

0 commit comments

Comments
 (0)