feat: have contract call compute the right cost call#7164
Conversation
to charge the right cost, we needed the ability to pass from and to caller/callee the cost before and after call.
|
|
||
| let in_mem_offset = offset + arg_size; | ||
|
|
||
| ensure_memory(&memory, &mut store.as_context_mut(), in_mem_offset as usize)?; |
There was a problem hiding this comment.
That would create a second usage of this function for each contract call. This could be costly. It's highly unlikely but we could have to recreate a new memory twice (we would need to call a function where the arguments size are bigger than a wasm page), but it's a possibility.
| // Access the global stack pointer from the instance | ||
| let stack_pointer = instance | ||
| .get_global(&mut store.as_context_mut(), "stack-pointer") | ||
| .ok_or(Error::Wasm(WasmError::GlobalNotFound( | ||
| "stack-pointer".to_string(), | ||
| )))?; | ||
|
|
||
| let offset = stack_pointer | ||
| .get(&mut store.as_context_mut()) | ||
| .i32() | ||
| .ok_or(Error::Wasm(WasmError::ValueTypeMismatch))?; | ||
|
|
||
| let memory = instance | ||
| .get_memory(&mut store.as_context_mut(), "memory") | ||
| .ok_or(Error::Wasm(WasmError::MemoryNotFound))?; |
There was a problem hiding this comment.
call_function will also compute all these. Pass them as parameter instead of recomputing them.
| number_of_arguments: u32, | ||
| total_size_of_parameters: u32, | ||
| ) -> Result<(), Error> { | ||
| let function_name = ".contract-call-cost-overhead"; |
There was a problem hiding this comment.
Pfffffffff... at worst, that's a constant, but this let is a bit excessive....
| // Charge the cost of contract call analog to the UserFunctionApplication and InnerTypeCheckCost in the interpreter. | ||
| // UserFunctionApplication charge proportionally to the number of arguments passed to the contract call. | ||
| // InnerTypeCheckCost charge proportionally to the cumulative size of each argument's type. | ||
| fn apply_contract_call_cost( |
There was a problem hiding this comment.
What is even the point of this function? It's recomputing most of the stuffs we already have in call_function, and then it calls the wasm function .contract-call-cost-overhead. Why not inline it and merge some operations (the lookup for memory and stack pointer, the ensure_memory call, the update of stack-pointer).
| name: &str, | ||
| value: Val, | ||
| ) -> Result<Global, Error> { | ||
| let store2 = store.as_context_mut(); |
| ) | ||
| .map_err(|e| Error::Wasm(WasmError::UnableToLoadModule(e)))?; | ||
|
|
||
| let mut store2 = store.as_context_mut(); |
| value: Val, | ||
| ) -> Result<Global, Error> { | ||
| let store2 = store.as_context_mut(); | ||
| let the_global = Global::new( |
| let runtime = link_global(linker, store, Cost::Runtime.as_str(), Val::I64(i64::MAX))?; | ||
| let read_count = link_global(linker, store, Cost::ReadCount.as_str(), Val::I64(i64::MAX))?; | ||
| let read_length = link_global(linker, store, Cost::ReadLength.as_str(), Val::I64(i64::MAX))?; | ||
| let write_count = link_global(linker, store, Cost::WriteCount.as_str(), Val::I64(i64::MAX))?; | ||
| let write_length = link_global( | ||
| linker, | ||
| store, | ||
| Cost::WriteLength.as_str(), | ||
| Val::I64(i64::MAX), | ||
| )?; |
There was a problem hiding this comment.
All those i64::MAX are the initial value for all these costs? And they aren't defined anywhere as constants? 😮
|
|
3 participants
Add changes for 798 in clarity. All context for these changes can be found in that PR