Added Nonce Enforcer as Caveat in Delegation Core#44
Conversation
| if (typeof nonce !== 'string' || !nonce.startsWith('0x')) { | ||
| throw new Error('Invalid nonce: must be a valid hex string'); | ||
| } |
There was a problem hiding this comment.
This needs to accept ArrayBuffer also, as that's included in the BytesLike type. The intention is the caller can decide whether to use ArrayBuffer or string - ideally we would perform the operation in whatever data type was specified - unless there's performance / overhead benefits to doing type conversion.
We could either:
- convert ArrayBuffer to string (and do the length fixing with the string)
- convert string to ArrayBuffer (and do the length fixing with the ArrayBuffer)
- add a
lengthparameter to theprepareResultfunction, so that we can just pass the raw nonce after validation
Also, can we drop this check entirely, as line 61 does the same check, but also validates the "body" of the hex string?
There was a problem hiding this comment.
Added, please take a look
| if (!nonce || nonce === '0x') { | ||
| throw new Error('Invalid nonce: must be a non-empty hex string'); | ||
| } |
There was a problem hiding this comment.
Because this needs to accept ArrayBuffer, we should consider how we want to validate that - should we throw if the value is a zero-length ArrayBuffer, or should we consider that 0x00 - "0x" has a specific meaning (null) which I don't think zero-length ArrayBuffer does?
There was a problem hiding this comment.
Added, please take a look
There was a problem hiding this comment.
can we please add some tests for ArrayBuffer input ?
There was a problem hiding this comment.
Added, please take a look
2 participants
📝 Description
Added the nonce caveat
createNonceTermsto the delegation-core🔄 What Changed?
List the specific changes made:
createNonceTermsto the delegation-core🚀 Why?
Explain the motivation behind these changes:
🧪 How to Test?
Describe how to test these changes:
List any breaking changes:
📋 Checklist
Check off completed items:
🔗 Related Issues
Link to related issues: https://app.zenhub.com/workspaces/readable-permissions-67982ce51eb4360029b2c1a1/issues/gh/metamask/delegator-readable-permissions/305
📚 Additional Notes
This changed required to tests the e2e tests related to the nonce caveat. After testing that an error was found for the e2e related to the function
stringToUnprefixedHexso it got deleted for all e2e tests. At this point other non-related e2e are failling