Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Public Spec: Callbacks as named, exported functions #51

Closed
wants to merge 2 commits into from

Conversation

roman-kashitsyn
Copy link
Contributor

Previously, callbacks were identified by indices in an exported table,
which are hard to keep stable across upgrades. By identifying callbacks
with names, and exposing them via named function exports, those
developers who aim to have stable callbacks across upgrades have a
reasonable chance.

Since we are touching this anyways, it also changes the size of the
env to i64; this does not affect the system-side implementation
greatly, but allows canisters to do interesting tricks, such as never
re-using an env value, or encoding both version information and a
32-bit counter in it.

This PR is an iteration of #183, which did away with “names” for
callbacks completely and only used the env value for dispatch, using a
single entry point, but subsequent discussion declared that as
undesirable.

Previously, callbacks were identified by indices in an exported table,
which are hard to keep stable across upgrades. By identifying callbacks
with names, and exposing them via named function exports, those
developers who aim to have stable callbacks across upgrades have a
reasonable chance.

Since we are touching this anyways, it also changes the size of the
`env` to `i64`; this does not affect the system-side implementation
greatly, but allows canisters to do interesting tricks, such as never
re-using an `env` value, or encoding both version information and a
32-bit counter in it.

This PR is an iteration of #183, which did away with “names” for
callbacks completely and only used the `env` value for dispatch, using a
single entry point, but subsequent discussion declared that as
undesirable.
@roman-kashitsyn roman-kashitsyn requested a review from a team as a code owner June 3, 2022 12:41
@netlify
Copy link

netlify bot commented Jun 3, 2022

Deploy Preview for ic-interface-spec ready!

Name Link
🔨 Latest commit 9141fe3
🔍 Latest deploy log https://app.netlify.com/sites/ic-interface-spec/deploys/62a1bb3085c7110008fcee9f
😎 Deploy Preview https://deploy-preview-51--ic-interface-spec.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@roman-kashitsyn
Copy link
Contributor Author

roman-kashitsyn commented Jun 3, 2022

@nomeata authored this proposal in the internal archived public spec repo. The patch didn't apply cleanly; I hope my resolution of conflicts didn't break anything.

@nomeata
Copy link
Contributor

nomeata commented Jun 3, 2022

Since we are touching this anyways, it also changes the size of the env to i64;

8 bytes ought to be enough for everyone?

At this point, I wonder if we should just allow blobs as environments. This allows canisters to put even more information into the callback without having to touch it’s own memory, which can for example be otherwise completely stateless. It’d have to be accounted for, of course, but that seems doable.

spec/index.adoc Outdated Show resolved Hide resolved
Co-authored-by: John Plevyak <[email protected]>
@Dfinity-Bjoern
Copy link
Member

To me, this looks like a breaking change to the canister API. Isn't it?

@Dfinity-Bjoern
Copy link
Member

Closing the PR for now because there is no active work and it is unclear that converting it to markdown is beneficial at this point. The PR can be converted and reopened at any point in time, such as when the engineering teams actually have capacity to work on the implementation.

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

Successfully merging this pull request may close these issues.

4 participants