[Infra] Add VerifySignatures method for use by SCs #509
Conversation
| public static class Operations | ||
| { | ||
| public static void Noop() { } | ||
| public static Address DeserializeAddress(byte[] address) |
There was a problem hiding this comment.
Why not copy the much more readable existing implementation?
There was a problem hiding this comment.
Also this would go in the Stratis.SCL.Serialization namespace.
| /// <param name="message">The message that was signed.</param> | ||
| /// <param name="addresses">If not <c>null</c> the addresses returned are intersected with these addresses.</param> | ||
| /// <returns>The list of addresses that produced the signatures and constrained to the list provided in <paramref name="addresses"/> if not <c>null</c>.</returns> | ||
| public static Address[] VerifySignatures(byte[] signatures, byte[] message, Address[] addresses = null) |
There was a problem hiding this comment.
What do you think of the following:
- Split this into 2 methods instead of allowing a null param for addresses
- Using the Try pattern instead of returning null ie.
bool TryVerifySignatures(..., out Address[] verifiedAddresses)
There was a problem hiding this comment.
We should also wrap everything in a try/catch (gross, I know, but NBitcoin especially likes to throw a lot) to prevent exceptions being propagated into the contract.
|
Would be nice to see tests similar to those in #414. |
|
Tests added. Additional tests to accompany the dictionary contract PR. |
quantumagi
changed the title
| /// <returns>An array of size N * <paramref name="subArrayLength"/></returns> | ||
| public static T[] FlattenArray<T>(T[][] array, int subArrayLength) where T : struct | ||
| { | ||
| if (array == null || array.Any(x => x == null || x.Length != subArrayLength)) |
There was a problem hiding this comment.
It is needed to validate parameters ? Because below logic will already fail if array is null or contains null items.
These are going to cost gas so code should to be efficient. You can try catch whole logic and return null. Secondly below code already iterate values so you may validate items there. Imo if it will fail already so no need to check there even.
@rowandh wdyt ?
There was a problem hiding this comment.
Could we perhaps make this private for now? Otherwise there's 3 decisions about the SCL API in this PR.
I'm fine with the implementation. I think we could do a full review of additional optimizations before SCL is published.
There was a problem hiding this comment.
@rowandh , I've removed flatten / unflatten in favour of using the ContractPrimitiveSerializer.ToArray from within the contract itself. That means we're now passing string[] signatures to VerifySignatures.
3 participants
No description provided.