search

dfinity / interface-spec

IC Interface Specification
https://khsfq-wqaaa-aaaak-qckvq-cai.icp0.io/docs
37 stars 20 forks source link

feat: Add a system api call to burn cycles #230

Closed dragoljub-duric closed 1 year ago

dragoljub-duric commented 1 year ago

This PR adds a new system API that canisters can use to burn cycles on demand. Such an API can be used by a burner canister that will represent usage on “private” subnets that are prepaid for as well as by the Bitcoin canister if we want to burn the cycles sent to it.

github-actions[bot] commented 1 year ago

🤖 Here's your preview: https://org5p-7iaaa-aaaak-qckna-cai.icp0.io/docs

dragoljub-duric commented 1 year ago

@mraszyk MR is now merged. Can you please approve this PR?

crusso commented 1 year ago

Do we actually need this new functionality? It seems to be that just creating a canister (with some cycles) and then deleting that canister will have the same effect. This can all be done with existing functionality in the management canister.

Here's an example of different methods, including a best one.

https://github.com/crusso/burn

https://github.com/crusso/burn/blob/main/src/burn/main.mo#L46

I haven't tried this code in a while so it's possible it no longer works. @mraszyk, will this still work?

It might also not work in the Motoko playground due to sandboxing of canisters: the playground intercepts management canister calls and cycle transfers to prevent third parties stealing cycles from the playground. (@chenyan-dfinity can advise).

dsarlis commented 1 year ago

Do we actually need this new functionality? It seems to be that just creating a canister (with some cycles) and then deleting that canister will have the same effect. This can all be done with existing functionality in the management canister.

Here's an example of different methods, including a best one.

https://github.com/crusso/burn

https://github.com/crusso/burn/blob/main/src/burn/main.mo#L46

I haven't tried this code in a while so it's possible it no longer works. @mraszyk, will this still work?

It might also not work in the Motoko playground due to sandboxing of canisters: the playground intercepts management canister calls and cycle transfers to prevent third parties stealing cycles from the playground. (@chenyan-dfinity can advise).

This seems like a big hack to me. Besides the conceptual point of abusing canister creation/deletion to get cycles appear as burned, I see some other problems as well:

  1. The fact that cycles are gone when you delete a canister now is something that could change in the future. IIRC, there were discussions to return the cycles to the controller (or one of the controllers) or even any principal that was designated by a controller (assuming it's a canister of course). I don't recall the details of why we did not go with this decision but it seems risky to indirectly depend on this behaviour.
  2. I hardly believe we want to encourage this behaviour of creating canisters with the sole purpose of killing them immediately after. Canister ids cannot be reused when a canister is deleted so this could turn into a huge problem if people abuse creating canisters that way and we start running out of canister ids.

An api that directly allows you to burn cycles seems more appropriate and something that has a predictable behaviour in the long run. It's something we have also discussed in the past in other contexts so it's not something entirely new or out of the question.

crusso commented 1 year ago

Do we actually need this new functionality? It seems to be that just creating a canister (with some cycles) and then deleting that canister will have the same effect. This can all be done with existing functionality in the management canister. Here's an example of different methods, including a best one. https://github.com/crusso/burn https://github.com/crusso/burn/blob/main/src/burn/main.mo#L46 I haven't tried this code in a while so it's possible it no longer works. @mraszyk, will this still work? It might also not work in the Motoko playground due to sandboxing of canisters: the playground intercepts management canister calls and cycle transfers to prevent third parties stealing cycles from the playground. (@chenyan-dfinity can advise).

This seems like a big hack to me. Besides the conceptual point of abusing canister creation/deletion to get cycles appear as burned, I see some other problems as well:

  1. The fact that cycles are gone when you delete a canister now is something that could change in the future. IIRC, there were discussions to return the cycles to the controller (or one of the controllers) or even any principal that was designated by a controller (assuming it's a canister of course). I don't recall the details of why we did not go with this decision but it seems risky to indirectly depend on this behaviour.
  2. I hardly believe we want to encourage this behaviour of creating canisters with the sole purpose of killing them immediately after. Canister ids cannot be reused when a canister is deleted so this could turn into a huge problem if people abuse creating canisters that way and we start running out of canister ids.

An api that directly allows you to burn cycles seems more appropriate and something that has a predictable behaviour in the long run. It's something we have also discussed in the past in other contexts so it's not something entirely new or out of the question.

Ok, I'll buy those arguments.