Skip to content

chore(canister_logging): update charging cycles tests#9785

Open
maksymar wants to merge 5 commits intomasterfrom
maksym/fix-lms-related-tests
Open

chore(canister_logging): update charging cycles tests#9785
maksymar wants to merge 5 commits intomasterfrom
maksym/fix-lms-related-tests

Conversation

@maksymar
Copy link
Copy Markdown
Contributor

@maksymar maksymar commented Apr 9, 2026

This PR updates charging cycles tests due to the recent change in pricing.

#9752

@github-actions github-actions bot added the test label Apr 9, 2026
@maksymar maksymar changed the title test: fix log memory store related tests chore(canister_logging): update charging cycles tests Apr 9, 2026
@github-actions github-actions bot added chore and removed test labels Apr 9, 2026
@maksymar maksymar marked this pull request as ready for review April 9, 2026 15:31
@maksymar maksymar requested a review from a team as a code owner April 9, 2026 15:31
// Memory at install time (~12_510 bytes with log, ~222 bytes without):
// 134 bytes canister history
// + 12_288 bytes log memory store (4 KiB header + 4 KiB index + 4 KiB data)
// + 88 bytes other system state overhead
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of curiosity: do you know what an empty canister has beyond canister history and logs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suppose that should be it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but then what are these 88 bytes?

migrated_canister,
if LOG_MEMORY_STORE_FEATURE_ENABLED {
9_200_000
11_200_000
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How was this derived? Just experimentally?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, just experimentally

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we could use the same "trick" as I suggested before (raise the freezing threshold) so that we don't need to compute the number of cycles so precisely. But that's beyond the scope of this PR so I don't insist on doing it right now.

Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com>
// the test. It needs to be _just_ enough to allow for the canister
// to be installed (the canister is created with the provisional
// create canister api that doesn't require additional cycles).
// The amount of cycles must be just enough for `install_code` to
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why "just enough"? It seems like this test only cares about the canister being frozen which can be achieved by setting the freezing threshold to a very high value (u64::MAX / 2 or whatever the system still allows) without computing the initial cycles balance so precisely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants