-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Problem
PR #33 introduces this comment:
// TODO: This option currently breaks `pendingKeyRequests` due to this
// being set on `OP_KEY_ADD`.
Context: we set _volatile.pendingKeyRequests on a contract to keep track of unanswered (i.e., pending) key requests that we've sent. However, we do this when we process an OP_KEY_ADD on our contract, which is part of how we send an OP_KEY_REQUEST (essentially, OP_KEY_REQUEST is sent to the contract we're making the request to, and OP_KEY_ADD is sent to our contract).
One of the reasons we do this on OP_KEY_ADD is that this allows us to keep track of the requests that we have sent (requests others have sent aren't of much interest) and because we include some additional fields (e.g., the request reference) which are relevant for us but not relevant for the party answering.
PR #33 introduces a way to send just the OP_KEY_REQUEST, without an OP_KEY_ADD (which is useful for cases such as re-requesting keys, since we've already added keys to our contract for the request to be answered). However, omitting the OP_KEY_ADD has the effect that we can't keep track of the request (which in turn makes it difficult to avoid sending duplicate requests).
Solution
We need to implement a way of keeping track of requests that doesn't depend on sending an OP_KEY_ADD. We could consider moving reference into the OP_KEY_REQUEST and then moving from _volatile to _vm (that is, keeping track of all requests). This would be a 'breaking' change in these regards:
- If we act on
OP_KEY_REQUESTinstead of onOP_KEY_ADD(in other words, remove the existing logic fromOP_KEY_ADD), then processing old events will not work as it does now. - Moving from
_volatileto_vm, without keeping the existing logic for handling_volatilewould break existing states (requiring contracts to be re-processed).
We can avoid the breaking parts by keeping the old logic for handling older messages / states and using the new shapes going forward.