Support EIP1271 signatures

[adamgall]

Work In Progress

Closes #31

This hasn't been tested or even executed yet. But it does build! Open to feedback as I continue implementation and testing.

Notes

This PR will make the implementation of #26 less straightforward than originally expected.

[jshufro]

can we change this to an if/else so that we can deduplicate the two calls to s.checkNodeAuthorization?

[adamgall]

Hmm, I don't think I follow how to make that happen.

The call to getNodeID(msg, sig) will not give an error if the signature was made with a contract, it will just recover a random incorrect address. We do need to call checkNodeAuthorization with that address to see if it is actually in the "whitelist".

The second call to checkNodeAuthorization below checks the eip1271 recovered address for it's inclusion on the whitelist.

I'm not sure how we would avoid making two calls to checkNodeAuthorization in the case that the signature was generated via eip1271.

If I'm missing the point of your request, please keep poking me in the right direction :)

[jshufro]

hmm yeah maybe not if/else, but

if err != nil {
  // Attempt EIP-1271 validation
  ...
  valid, err := ...
  if err != nil {
    // return error
  }
}
 // checkNodeAuthz using either the EOA address or SC address

[adamgall]

But how would we know to check with either the EOA address or the SC address? We need to check with both. Either neither of them will pass checkNodeAuthz, or one of them will pass checkNodeAuth. AFAIK it's not possible to know which one is "valid" until we do the actual check.

[jshufro]

As mentioned offline- checking the EOA sig needs to return an error if the recovered address doesn't match the one in the json payload. I think it's possible for getNodeID to return a valid address for an EIP1271 signature, just essentially random bytes.

Once we have that validation, we know a priori whether an EOA signed the message, and we can implement this as a more "pure" bit of fallback logic (ie, first check EOA signature, if not valid, check EIP1271, and use the appropriate address in a single checkNodeAuthorization call.)

[jshufro]

Will close #26

[jshufro]

out.sig is a []byte that represents binary, so we need to hex-encode before logging.

Msg is expected to be valid ascii, so the cast is fine there.

[jshufro]

As mentioned offline- checking the EOA sig needs to return an error if the recovered address doesn't match the one in the json payload. I think it's possible for getNodeID to return a valid address for an EIP1271 signature, just essentially random bytes.

Once we have that validation, we know a priori whether an EOA signed the message, and we can implement this as a more "pure" bit of fallback logic (ie, first check EOA signature, if not valid, check EIP1271, and use the appropriate address in a single checkNodeAuthorization call.)

[jshufro]

Will close #26

[adamgall]

Alright @jshufro! Ready for another round of review :)

[jshufro]

one small bug and it's good to go

[jshufro]

need to parse hex here- you can use https://pkg.go.dev/github.com/ethereum/[email protected]/common/hexutil#Decode

[adamgall]

updated and pushed

[jshufro]

since we're gonna use the function from my previous comment, use https://pkg.go.dev/github.com/ethereum/[email protected]/common/hexutil#Encode here to restore the 0x prefix

[adamgall]

updated and pushed

[jshufro]

Seems like the grpc lint errors are from the dependency bump, so it falls on you to see about fixing them

[adamgall]

Seems like the grpc lint errors are from the dependency bump, so it falls on you to see about fixing them

Ok, I'll look into this. I wasn't aware that there were grpc lint errors! Thanks for your patience as I make my way into golang development 😅