Realizing that this is more than just validating signatures; we're actually asking the question "given this action, does the caller have the ability to it, given this proof"?

so perhaps the better metaphor is

isValidAction(bytes _action, bytes _proof) {}

where the default behavior might be to treat _action as a Bouncer data hash and _proof as a signature. But like we noticed before, they could be anything.

Although this implies that the proof handling and the access control are part of the same function/contract where they could potentially be different, although I'm not sure how that separation would generalize at all (i.e. in the case of ECDSA signatures, the validity is just the recovery, which produces an address and the access control is a "does this address have this permission" check against the _action data).

Thoughts, @PhABC?

That's a good point! A signature is usually used to guarantee that a given address allows something, but other schemes not relying on signature methods could also achieve the same goal.

Pinging @alexvandesande & @abandeali1 for comments.

I personally prefer using a 32 byte hash over an _action byte array. Any action should be able to be represented as a hash, and this really simplifies the implementation details. You're probably going to end up hashing _action and verifying a signature at some point anyways. Any extra logic could live in the caller contract.

I could see it making sense to use the word proof instead of signature, though.

@abandeali1

The reason why we opted for a dynamic byte array is because the called contract might request specific data that the caller contract is not aware of.

For instance, imagine you use a smart account that has some key management properties, where different keys have different roles (RBAC style). Now imagine that the private key on your phone can sell some game assets, but can't any other types of tokens. Your ledger private key can sell any type of asset. If the caller contract (e.g. 0x contract) only passes the hash of the message, the smart account has no way of knowing what the asset being traded is. The smart account would see two hashes that were indeed signed by two private key you own, but it would have no way of verifying if the action signed is valid based on it's own internal conditions. In this case, your smart account would either need to allow all your private keys to exchange assets via 0x (let them sign hashes) or prevent them all.

In general, what i'm trying to say is that I think there could always be additional rules not encompassed by the caller contract that the called contract requires.

Likewise, my worry is that a hash loses information that may be necessary for validation.

That said, I'm also worried about the complexity of passing arbitrary data via a bytes call; we've just accidentally re-created the need for abi encoding/decoding and whoops, we should probably just use solidity for that, so now we're back where we started, creating context-specific methods.
It may make more sense to simply restrict this to hash + proof validation, and nothing more? that covers bouncer and identity contracts, the two main cases I'd like to use this technique for.

You can always encode any extra data about the hash in the signature field. For example:

function isValidSignature(bytes32 hash, bytes signature) external {
    uint8 signatureType = uint8(signature[0]);
    bytes32 msgHash = keccak256(hash, signatureType);
    if (signatureType == 0) {
        // Do something specific to signatureType
        return _isValidSignature(msgHash, signature);
    } else if ...
}

You can imagine adding any amount of extra data to signature in a similar way. This keeps the simplest implementation simple, but it's still pretty flexible if necessary.

@abandeali1 I'm not convinced that overloading the signature ("proof") argument is a ton better than bytes action, bytes proof.

@PhABC any feedback on restricting this to hash+signature based validation?

@shrugs my point is that it can be done with just a hash+signature, but I wouldn't expect that to be frequently used.

Thanks for writing this up @PhABC, it's quite good.

The discussion around @abandeali1's suggestion is very interesting, but we need to show some examples of usage in order to best decide on fixed or arbitrary size _data (i.e. the message that is signed).

I think a good example to analyze is that of an m-of-n multisig smart account. A valid signature for this account could be the concatenation of m (signature, signer) pairs, where the m signers are in the set of n signers, and each signature is valid for the message and respective signer. I think this is enough motivation for the signature to be of arbitrary size.

Now suppose that a family of messages can only be considered signed when the full set of n signers approve. It's common to increase the required number of signatures for large value transfers, so I think it would make sense for the signature scheme to have something analogous to that. The validity of the signature then depends on some parameter in the message being signed. Per @abandeali1's suggestion, the parameter should be included as part of the signature, but then the contract would have to additionally verify that the parameter in the signature is indeed the one contained in the message. If the message is hashed, then the entire message has to also be included in the signature in order to verify the hash. That would be a lot of redundant bytes being passed around. IMO this justifies having the message be of arbitrary size, and not necessarily a hash.

Do you all agree the scenario described makes sense?

A mostly unrelated question about the EIP. Would it be valid for an implementation of isValidSignature to change its behavior depending on the caller? I have the feeling that this should be forbidden by the spec ("MUST NOT"), but unfortunately I don't think there's a way to enforce it in the API.

I forgot to comment on the suggestion to use the names "action" and "proof". Although I kind of like "proof", I would prefer to keep the current names as they're consistent with those of cryptographic signing, and agnostic to any particular use case.

@frangio overloading the signature field should actually result in fewer bytes being passed around and should be cheaper, assuming that isValidSignature is being called by another contract (i.e the data is being copied to memory and is not included in the transaction calldata).

With bytes data, bytes signature, the ABI encoded bytes will look like: [dataOffset][signatureOffset][dataLength][data][signatureLength][signature], where everything is 32 bytes except for data and signature. This gives us a total of 128 + data.length + signature.length bytes.

With bytes32 hash, bytes signature, the ABI encoded bytes will look like: [hash][signatureOffset][signatureLength][signature]. This gives us a total of 96 + data.length + signature.length bytes (assuming that the signature field includes everything that would otherwise be in the data field).

So in the worst case scenario of using bytes32 hash, bytes signature, we save 32 bytes but add some extra hashing and complexity.

Now let's look that the case where we we use bytes data, bytes signature but data only contains a 32 bytes hash. The ABI encoded bytes are [dataOffset][signatureOffset][dataLength][hash][signatureLength][signature]. This means we add 64 bytes vs using bytes32 hash, bytes signature. There's an extra hidden cost here though - if calling isValidSignature from another contract, we now have to convert our 32 byte hash into a byte array before the call and then parse the hash out of byte array after the call. This isn't particularly easy without using some inline assembly (currently), and I'm guessing that the function will be used this way the majority of the time.

To be honest, the cost differences here are probably minimal. I think the main downside for the bytes data, bytes signature approach is converting a bytes32 to and from a byte array. With the bytes32 hash, bytes signature approach, the downside is confusion over potentially overloading signature. Which is better likely comes down to how often we think each case is likely to be used.