Skip to content

Conversation

@DashKash54
Copy link
Collaborator

@DashKash54 DashKash54 commented Dec 10, 2024

Description

Tests for supporting EIP1271 AuthSig (not SessionSig). The required node PR has already been merged and deployed to Datil-dev: https://github.com/LIT-Protocol/lit-assets/pull/1629.

Test Suite

Two Tinny tests which test calling the EIP1271 isValidSignature() deployed on Yellowstone:

  1. The contract simply checks whether the recovered address equals the tempOwner. Anyone can set the tempOwner and this is allowed since there is no guarantee which wallet is used when executing the tests.
  2. Initially a random address, deployerAddress, is set as the tempOwner to demonstrate the failure case i.e. when the isValidSignature() function returns 0xffffffff.
  3. The node will throw an error: validation error: Authsig failed for contract ${contractAddress}. Return value was ffffffff.
  4. We assign Alice to be the tempOwner.
  5. Alice creates an AuthSig by hashing the SIWE message and signing it with either signMessage() or signDigest() depending on whether it's signing a Keccak hash or SHA256 one.
  6. Call the smart contract directly with the siweMessageHash and signature to ensure that the isVerifySignature() returns the MAGICVALUE = 0x1626ba7e.
  7. Finally, calls the Lit's decrypt function and successfully decrypts the secret message: 'Decrypted from EIP1271 AuthSig'.

Access Control Condition

{ 
    contract_address: "", 
    chain: "yellowstone", 
    standard_contract_type: "", 
    method: "", 
    parameters: [":userAddress"], 
    return_value_test: { comparator: "=", value: "0x88105De2349f59767278Fd15c0858f806c08d615" },
}

(EIP1271) AuthSig

{
  sig: "****filtered****", 
  derived_via: "EIP1271", 
  signed_message: "localhost wants you to sign in with your Ethereum account:\n0x25Da42DBB1a41eEb8D1Ec3d8cEa4545BBea0D949\n\nThis is a test statement.  You can put anything you want here.\n\nURI: https://localhost/login\nVersion: 1\nChain ID: 1\nNonce: 0x21036c8dbbea5fd939b6a49ee3e3814b8070668bba46211aeea0f6521e6ad4ad\nIssued At: 2024-12-10T10:50:23.539Z\nExpiration Time: 2024-12-17T10:50:23.539Z", 
  address: "0x88105De2349f59767278Fd15c0858f806c08d615",  // contractAddress
  algo: None, 
  auth_material_type: ContractSig, 
  chain: None, // Not used since only the chain mentioned in the Access Control Condition is used to determine whether the provided AuthSig is permitted
}

Test Cases

  1. Hash the Ethereum Signed Message, this happens internally in the ethers.utils.signMessage() function and pass it with derivedVia: EIP1271
  2. Hash the bare message without any prefix using the crypto packages SHA256 hash. This is similar to what's used in WebAuthn and pass it with derivedVia: EIP1271_SHA256

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@DashKash54 DashKash54 requested a review from Ansonhkg as a code owner December 10, 2024 11:55
@DashKash54 DashKash54 changed the title Feature/lit 4041 support eip1271 on the sdk Feature/lit 4041 Support eip1271 on the sdk Dec 10, 2024
@DashKash54 DashKash54 changed the title Feature/lit 4041 Support eip1271 on the sdk Feature/lit 4041 Support Eip1271 Dec 10, 2024
);
try {
const isValid = await contract.isValidSignature(hash, siweSignature);
if (isValid === '0x1626ba7e') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isValid === '0x1626ba7e') {
if (isValid !== '0xffffffff') {

The error message indicates this is the correct condition. However, as EIP-1271 only strictly defines the magic value for a valid signature, I would say the check should always have to be over that one. I think the real correction is to change the error message to something like Expected isValidSignature to not be 0x1626ba7e (valid signature)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I think the whole point of comparing the exact value is that it is very specific 0x1626ba7e, and its use follows an unofficial yet widely accepted convention for implementing EIP-1271. Removing it would reduce the specificity and clarity for this "standard"

);
try {
const isValid = await contract.isValidSignature(hash, siweSignature);
if (isValid === '0x1626ba7e') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isValid === '0x1626ba7e') {
if (isValid !== '0xffffffff') {

Same as above

@DashKash54 DashKash54 merged commit 683a0d6 into master Dec 11, 2024
4 of 5 checks passed
@DashKash54 DashKash54 deleted the feature/lit-4041-support-eip1271-on-the-sdk branch December 11, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants