dfinity / wg-identity-authentication

Repository of the Identity and Wallet Standards Working Group
https://wiki.internetcomputer.org/wiki/Identity_%26_Authentication
Apache License 2.0
26 stars 8 forks source link

ICRC-39 batch sequential id requirement #136

Closed sea-snake closed 3 months ago

sea-snake commented 5 months ago

I read the following in the ICRC-39 spec while implementing it:

Order all requests in the batch by their id in ascending order.

When I read the JSON RPC spec, this requirement does not exist, it even defines that the responses could be out of order and ids should be used:

The Response objects being returned from a batch call MAY be returned in any order within the Array. The Client SHOULD match contexts between the set of Request objects and the resulting set of Response objects based on the id member within each Object.

Also since ids could also be strings besides numbers in the JSON RPC spec and the signer will receive the batch in correct order already since it's a list of requests, I would opt to not have this ascending order requirement on the ids.

frederikrothenberger commented 5 months ago

@sea-snake: The JSON-RPC spec also states:

The Server MAY process a batch rpc call as a set of concurrent tasks, processing them in any order and with any width of parallelism.

which we absolutely don't want. Batches will be used to process transactions with dependencies on each other. To avoid this footgun, we better have very clearly defined semantics on how to process the messages.

If we remove the ordering requirement then we rely on the array ordering, which is only implicit. Honestly, maybe it would be best to mandate the client send the batch ordered, and reject if the array order does not match the id ordering...

Or abandon the idea of piggy-backing on top of JSON-RPC batches altogether as it might cause more problems than it solves.

I'm really glad we split this part out exactly because of stuff like that... 😅

Are you already implementing batch support in the slide wallet?