Are there any important differences that people should consider if they migrate from ScopeLift's implementation to this one, or if an interface has to support both? I think it may be worth listing them here.

So far:

• The reference implementation is in OpenZeppelin Contracts v4
• It includes a nonce in the params object given v4 had no nonce protection
• The COUNTING_MODDE is different

The voting format may vary after the reviews. I think it's currently fine since it reads it's "based on", so no compatibility should be assumed right away

This function concerns me because the length of data is ignored and pos may be reading out of bounds. The assembly block correctly omits the memory-safe annotation, but like I said in a recent issue this globally disables optimizations so it should be avoided.

Is it possible to just check the validity of pos against the data length? Sadly I do see it would make the code less efficient by checking the length three times... Perhaps the helper should directly unpack all three parameters.

I think having the helper to unpack all three parameters is the easiest to ensure memory safetyness.

I'll update with this alternative

This is private, and everywhere we call it we first validate the length. Is that not good enough?

GIven how frequent it is for people to copy-paste from our code I think we should try to make functions self-contained and well documented even if they're private.

In this case, not only that I think this is the cleanest way of making the assembly block memory-safe to avoid disabling optimizations, but also I share the concern with @frangio; although the function is unreachable with invalid parameters, I don't feel comfortable having such a function around.

For the record, we both feel fine by keeping this function self contained

Similarly, we agreed that this double assignment is clear enough