refactor: Remove misleading callback_id from register_callback() test function

[stiegerc]

The function register_callback() takes a CallbackId as an argument. This suggests that this CallbackId is registered in the CallContextManager such that a response using this CallbackId can later be inducted successfully by passing this sanity check.

This is false. The CallbackId handed to register_callback() is used to construct an update call, but the origin of a call is irrelevant for the sanity check because each message gets its own CallbackId. It is in general not possible to ask the CallContextManager to register a CallbackId, rather you must ask the CallContextManager to register a new Callback for your call and then the CallContextManager generates and registers this Callback with a new CallbackId and returns it to you. This is the CallbackId that must be used to generate the message to pass the sanity check.

Since the origin of the call is irrelevant for the intended purpose of this function, it makes sense to use a system task as the call origin (i.e. a heartbeat or a timer), since that does not require a CallbackId as an input and return the CallbackId that must be used to construct the message instead.

The changes in this PR are such that the misleading CallbackId argument is removed, but otherwise the code is intact. Further changes to improve consistency with how the CallContextManager should be used will be done in a different PR.

[dsarlis]

Is this supposed to be a draft PR or is it really ready for review?

[stiegerc]

On Gitlab people didn't get a notification when you put Draft: in front. Apparently this works differently somehow on Github. There are some things I was going to do before it's ready. Sorry for the confusion.

[stiegerc]

This is where the sanity check is done when inducting responses (called from the system state here).

What it checks is the callback ID that is registered here, not what we passed into register_callback() before this change. IIUC CallOrigin::Update has to do with downstream calls so it's the callback ID above in the call tree, which is completely irrelevant for the sanity check AFAICT.
The first callback ID that register_callback() registers and returns is 1 because next_callback_id is initialized to 0.

So the reason tests using register_callback() appear to be working is that they use a callback ID of 1 as the default for test messages, e.g. here, here and here.

This is clearly not how it should be and quite misleading. It may also break on seemingly unrelated changes in the future.

[derlerd-dfinity]

Thanks a lot. LGTM.