[not FINAL] feat: new System API call ic0.call_cycles_add128_up_to #316
Conversation
|
🤖 Here's your preview: https://nlbfx-nyaaa-aaaak-qcnzq-cai.icp0.io/docs |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Dfinity-Bjoern
changed the title
| es.params.sysenv.compute_allocation, | ||
| es.params.sysenv.memory_allocation, | ||
| es.params.sysenv.freezing_threshold, | ||
| memory_usage_wasm_state(es.wasm_state) + es.params.sysenv.memory_usage_raw_module + es.params.sysenv.memory_usage_canister_history, |
There was a problem hiding this comment.
| memory_usage_wasm_state(es.wasm_state) + es.params.sysenv.memory_usage_raw_module + es.params.sysenv.memory_usage_canister_history, | |
| memory_usage_wasm_state(es.wasm_state) + es.params.sysenv.memory_usage_raw_module + es.params.sysenv.memory_usage_canister_history + max(|es.pending_call.method_name| + |es.pending_call.arg|, MAX_RESPONSE_SIZE), |
There was a problem hiding this comment.
Maybe not needed though as we don't model message memory anyway.
Introduces `call_cycles_add128_up_to` to the system API to offer a way of adding as many cycles as possible to a call without trapping in the subsequent `call_perform`. Corresponding spec MR: dfinity/interface-spec#316 Closes [SDK-1761](https://dfinity.atlassian.net/browse/SDK-1761?atlOrigin=eyJpIjoiY2QyZTJiZDRkNGZhNGZlMWI3NzRkNTBmZmVlNzNiZTkiLCJwIjoianN3LWdpdGxhYi1pbnQifQ) [SDK-1761]: https://dfinity.atlassian.net/browse/SDK-1761?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Introduces `call_cycles_add128_up_to` to the system API to offer a way of adding as many cycles as possible to a call without trapping in the subsequent `call_perform`. Corresponding spec MR: dfinity/interface-spec#316 Closes [SDK-1761](https://dfinity.atlassian.net/browse/SDK-1761?atlOrigin=eyJpIjoiY2QyZTJiZDRkNGZhNGZlMWI3NzRkNTBmZmVlNzNiZTkiLCJwIjoianN3LWdpdGxhYi1pbnQifQ) [SDK-1761]: https://dfinity.atlassian.net/browse/SDK-1761?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
| - It moves no more cycles than represented by a 128-bit value which can be obtained by combining the `max_amount_high` and `max_amount_low` parameters. | ||
|
|
||
| - A subsequent `ic0.call_perform` does not fail because of insufficient cycles balance | ||
| (if no extra bytes were added to the message via `ic0.call_data_append` between `ic0.call_cycles_add128_up_to` and `ic0.call_perform`) |
There was a problem hiding this comment.
| (if no extra bytes were added to the message via `ic0.call_data_append` between `ic0.call_cycles_add128_up_to` and `ic0.call_perform`) | |
| (if the canister makes no heap or stable memory allocations and no extra bytes were added to the message via `ic0.call_data_append` between `ic0.call_cycles_add128_up_to` and `ic0.call_perform`) |
There was a problem hiding this comment.
@dsarlis this seems a bit unpredictable to use. In higher-level languages, it seems difficult to guarantee that no extra memory is allocated. Should we maybe extend the implementation so that we can drop the additional conditions (by doing the computation of actual cycles to be sent to call_perform)?
There was a problem hiding this comment.
@mraszyk just pointed out that this would violate
This system call also copies the actual amount of cycles that were moved onto the call represented [...]
which seems to make it necessary to eagerly compute the amount of cycles attached. Not sure which is the best path forward here.
There was a problem hiding this comment.
It is possible to compute the actual amount of cycles attached by taking the difference of the canister's cycles balance before and after calling the system API ic0.call_cycles_add128_up_to (confirmed empirically). So dropping the destination pointer should not make the API substantially harder to use. The "trick" with computing the cycles balance difference does not work across ic0.call_perform though (as xnet fees are charged, too). Moreover, it is not clear what the semantics of multiple ic0.call_cycles_add128_up_to should be if we only attach the cycles during ic0.call_perform (the sum vs the maximum?). Hence, delaying the actual attachment to ic0.call_perform might not be the best path forward either.
I'd suggest to simply drop the destination pointer if the CDK teams could confirm that allocations are unlikely in between ic0.call_cycles_add128_up_to and ic0.call_perform. (Allocations happening after ic0.call_perform are fine as long as they are covered by the unused execution cycles that are refunded first.)
Dfinity-Bjoern
changed the title
|
Removing the |
Introduces `call_cycles_add128_up_to` to the system API to offer a way of adding as many cycles as possible to a call without trapping in the subsequent `call_perform`. Corresponding spec MR: dfinity/interface-spec#316 Closes [SDK-1761](https://dfinity.atlassian.net/browse/SDK-1761?atlOrigin=eyJpIjoiY2QyZTJiZDRkNGZhNGZlMWI3NzRkNTBmZmVlNzNiZTkiLCJwIjoianN3LWdpdGxhYi1pbnQifQ) [SDK-1761]: https://dfinity.atlassian.net/browse/SDK-1761?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
This PR adds a new System API call
ic0.call_cycles_add128_up_tomoving the maximum amount of cycles from the canister cycles balance onto an inter-canister call up to a specified upper bound.