-
Notifications
You must be signed in to change notification settings - Fork 4
feat: prevent HTTPs outcalls when stopping #76
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
feat: prevent HTTPs outcalls when stopping #76
Conversation
lpahlavi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @gregorydemay, LGTM! Just some very minor nits for the tests from me.
|
Thanks @gregorydemay! The ticket DEFI-2566 suggests adding a configuration option to the library, but I don't see that in this PR. I am thinking of potential use cases where a canister has some higher-level logic regarding HTTP outcalls that it may want to complete before stopping, in which case a configuration option would be useful for situations where the canister can be trusted to make the decision on whether or not to make HTTP outcalls while in the |
|
Good question @mbjorkqvist !
I had thought about this (initial commit had some new configuration field in |
Sometimes a canister needs to make a dynamic number of HTTPs outcalls (e.g. scraping Ethereum logs) and this can prevent the canister from being stopped and hence upgraded. This is because when
canister_stopis called, the canister is first entering thestoppingstate, which does prevent new calls to that canister, but does not prevent the canister itself from issuing new calls (see the specification on stop_canister).This PR therefore introduces a new middleware
CanisterReadyServicethat synchronously checks that the canister is running before delegating to the inner service.