Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: burning cycles programmatically #4690

Merged
merged 13 commits into from
Sep 13, 2024
Merged

feat: burning cycles programmatically #4690

merged 13 commits into from
Sep 13, 2024

Conversation

ggreif
Copy link
Contributor

@ggreif ggreif commented Sep 12, 2024 •
edited
Loading

This adds a new primitive (prim "cyclesBurn") and a prim.mo function cyclesBurn : <system> Nat -> Nat. It can be used to burn (some of the) canister's cycles programmatically using the system interface ic0.cycles_burn128.

@ggreif ggreif marked this pull request as ready for review September 12, 2024 17:25
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 12, 2024 •
edited
Loading

Comparing from ddf5299 to 42819c5:
In terms of gas, no changes are observed in 5 tests.
In terms of size, 5 tests regressed and the mean change is +0.0%.

@ggreif ggreif self-assigned this Sep 13, 2024
@ggreif ggreif added the feature New feature or request label Sep 13, 2024
luc-blaeser
luc-blaeser approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@luc-blaeser luc-blaeser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This looks good!

@ggreif ggreif added the automerge-squash When ready, merge (using squash) label Sep 13, 2024
crusso
crusso reviewed Sep 13, 2024
View reviewed changes
Copy link
Contributor

@crusso crusso left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the interpreters? Did we just crash on other cycles prims or do we need to adjust?

UPDATE: I checked the code and both interpreters just crash on cycles prims, so this is no worse. No need to fix now.

crusso
crusso reviewed Sep 13, 2024
View reviewed changes
src/codegen/compile_enhanced.ml
@@ -5338,7 +5338,7 @@ module Cycles = struct

let balance env =
Func.share_code0 Func.Always env "cycle_balance" [I64Type] (fun env ->
Stack.with_words env "dst" 4L (fun get_dst ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch (here and below). I guess we were over stack allocating here ...

crusso
crusso approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for the quick implementation!

@mergify mergify bot merged commit 1511fba into master Sep 13, 2024
10 checks passed
@mergify mergify bot removed the automerge-squash When ready, merge (using squash) label Sep 13, 2024
@mergify mergify bot deleted the gabor/burn branch September 13, 2024 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luc-blaeser luc-blaeser luc-blaeser approved these changes

@crusso crusso crusso approved these changes

Assignees

@ggreif ggreif

Labels
feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ggreif @crusso @luc-blaeser