ERCs icon indicating copy to clipboard operation
ERCs copied to clipboard

Update ERC-2098: variable v is not right in example.

Open chen4903 opened this issue 1 year ago • 4 comments

I have read and tried using the example of EIP-2098, but I have found some problems. I poc this error in solidity:

// SPDX-License-Identifier: GPL-3.0

pragma solidity =0.8.23;

contract TestEIP2098{
    function to_compact(uint256 r, uint256 s, uint256 v) public pure returns(uint256, uint256){
        // example 1
        // input:
        // r        = 0x68a020a209d3d56c46f38cc50a33f704f4a9a10a59377f8dd762ac66910e9b90
        // s        = 0x7e865ad05c4035ab5792787d4a0297a43617ae897930a6fe4d822b8faea52064
        // v        = 27
        // output:
        // r                  = 0x68a020a209d3d56c46f38cc50a33f704f4a9a10a59377f8dd762ac66910e9b90
        // yParityAndS here   = 0xfe865ad05c4035ab5792787d4a0297a43617ae897930a6fe4d822b8faea52064 (true)
        // yParityAndS in EIP = 0x7e865ad05c4035ab5792787d4a0297a43617ae897930a6fe4d822b8faea52064 (false)

        // example 2
        // input:
        // r        = 0x9328da16089fcba9bececa81663203989f2df5fe1faa6291a45381c81bd17f76
        // s        = 0x139c6d6b623b42da56557e5e734a43dc83345ddfadec52cbe24d0cc64f550793
        // v        = 28
        // output:
        // r                  = 0x9328da16089fcba9bececa81663203989f2df5fe1faa6291a45381c81bd17f76
        // yParityAndS here   = 0x139c6d6b623b42da56557e5e734a43dc83345ddfadec52cbe24d0cc64f550793 (true)
        // yParityAndS in EIP = 0x939c6d6b623b42da56557e5e734a43dc83345ddfadec52cbe24d0cc64f550793 (false)

        // But if you set v to 28 in example 1, you can get the same result in EIP. 
        // The same as example2, if you set v to 27, you can get the same result in EIP. 

        return (r, (v << 255) | s);
    } 
}

So I think there is some problem with variable v in the example of EIP-2098. Modify the value of v: 27=>28 in example 1, 28=>27 in example2.

chen4903 avatar Mar 23 '24 10:03 chen4903

File ERCS/erc-2098.md

Requires 1 more reviewers from @ricmoo Requires 3 more reviewers from @axic, @g11tech, @gcolvin, @lightclient, @samwilsn, @xinbenlv

eip-review-bot avatar Mar 23 '24 10:03 eip-review-bot

I think we need to get an expert, like @ricmoo, to check this.

SamWilsn avatar Apr 10 '24 17:04 SamWilsn

Thanks for the ping. I’ll look into it shortly. I At first look, it looks like they are correct.

ricmoo avatar Apr 10 '24 17:04 ricmoo

How about at second look? :innocent:

SamWilsn avatar May 29 '24 18:05 SamWilsn

@chen4903 can you please provide a program that we can verify the proposed signature is correct.

lightclient avatar Jul 17 '24 14:07 lightclient

The commit 1ebed1cdbfeffa1ea0a0a008a0c98ca4715af123 (as a parent of c8f8bcb279c33dc2b584ff2d6507d298c1380dd6) contains errors. Please inspect the Run Summary for details.

github-actions[bot] avatar Aug 08 '24 10:08 github-actions[bot]