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.

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

fix: [EXC-1836] Update hook status after update of canister settings #3624

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dragoljub-duric
Copy link
Contributor

@dragoljub-duric dragoljub-duric commented Jan 27, 2025

The PR adds a check condition for the low_wasm_memory hook when canister settings are updated because changes of wasm_memory_threshold or wasm_memory_limit or memory_allocation can trigger or un-trigger hook condition.

@github-actions github-actions bot added the fix label Jan 27, 2025
call_id: _,
message: _,
instructions_used,
result,
} => (result, instructions_used, Some(canister)),
} => {
canister.update_on_low_wasm_memory_hook_condition();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is required to mimic the behavior of execute_install_code because execute_install_code updates hook status in the process_install_code result. The reason is that install_code is only used in tests, and should behave similarly.

@dragoljub-duric dragoljub-duric marked this pull request as ready for review January 27, 2025 13:08
@dragoljub-duric dragoljub-duric requested a review from a team as a code owner January 27, 2025 13:08
@dragoljub-duric dragoljub-duric changed the title fix: Update hook status after setting wasm_memory_threshold fix: Update hook status after update of canister settings. Jan 27, 2025
@dragoljub-duric dragoljub-duric changed the title fix: Update hook status after update of canister settings. fix: [EXC-1836] Update hook status after update of canister settings Jan 27, 2025
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.

1 participant