Skip to content

feat: implement system cost primitives #5001

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.

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

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Kamirus
Copy link
Contributor

@Kamirus Kamirus commented Apr 3, 2025

No description provided.

@ggreif ggreif changed the title Implement system cost primitives feat: implement system cost primitives Apr 3, 2025
print(debug_show(before0) # " -- Cycles.balance() before0");
print(debug_show(before1) # " -- Cycles.balance() before1");
print(debug_show(after) # " -- Cycles.balance() after");
print(debug_show (before1 - after : Nat) # " -- Cycles.balance() diff");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Output from --playground

client: 500_000_000_000 -- create canister cost
client: created canister id: 7gngh-jqaaa-aaaab-qacvq-cai
client: 593_554_306_953 -- Cycles.balance() before0
client: 593_554_306_953 -- Cycles.balance() before1
client: 593_548_861_792 -- Cycles.balance() after
client:       5_445_161 -- Cycles.balance() diff

Copy link
Contributor Author

@Kamirus Kamirus Apr 4, 2025

Choose a reason for hiding this comment

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

500B is the documented cost, but why is the actual cost around 5M instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Why the reported canister cost is different than the actual amount?
  2. Why (with cycles = ) was not needed here? Cycles were automatically deducted from the balance.
    Whereas on the local replica this call was failing with not enough available cycles.


// let before = Cycles.balance();
printCycles();
let { canister_id } = await (with cycles = 500_000_000_000) ic00.create_canister({ settings = null });
Copy link
Contributor Author

@Kamirus Kamirus Apr 7, 2025

Choose a reason for hiding this comment

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

Output from the local replica:

38_461_538_461 -- create canister cost
already topped up; balance = 3_058_376_185_964
Cycles.balance()   = 3_058_376_185_964
Cycles.available() = 0
Cycles.refunded()  = 0
created canister id: be2us-64aaa-aaaaa-qaabq-cai
Cycles.balance()   = 2_558_375_756_574
Cycles.available() = 0
Cycles.refunded()  = 0
  1. The actual create canister cost is 38_461_538_461, which is smaller than the documented 500M, but this is probably a good sign
  2. (with cycles = ) is required for the call to succeed. Why does it not deduct from the balance automatically?
  3. Extra cycles are not refunded, is this expected? Are they lost?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about 1 though....

Copy link
Contributor Author

@Kamirus Kamirus Apr 8, 2025

Choose a reason for hiding this comment

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

Ad 3.

All cycles on this call are now the canister's initial cycles.

Ok, so this might explain why cycles are not returned 👍

Ad 1. @alexandru-uta found the answer: The actual cost on a local replica is 1/13 of the documented value

Also Ad 2. My question was: Why (with cycles = ) were not needed for the drun test? It might be because what Alex found - that drun test run as a system subnet 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Kamirus @crusso -- shall we invest some time into running drun as app subnet? I can take a stab at it since my very initial test did not show any run-drun tests failing..

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.

Took a quick pass (not a review) and it looks like you are doing all the right things... Nice!

Kamirus added 5 commits April 11, 2025 10:39
- Updated the `msg_cycles_accept128` function import to use the correct type.
- Introduced a new `cost_call` function in the `Cost` module to handle cycle costs for calls.
- Refactored existing cost functions for better clarity and organization.
Comment on lines +34 to +38
// 1_603_066_000 -- http cost request
// 1_603_060_000 -- http cost request (with requestSize = 0)
// 1_603_260_000 -- http cost request (with requestSize = 500)
// 1_603_660_000 -- http cost request (with requestSize = 1500)
// 2_003_060_000 -- http cost request (with requestSize = 1_000_000)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really accurate...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants