Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 fix inlined ec_Dbl in shamir-strauss #48

Merged
merged 1 commit into from
Dec 15, 2023
Merged

🐛 fix inlined ec_Dbl in shamir-strauss #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/ECDSA256r1.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
/// @param scalar_u Multiplier for basepoint G
/// @param scalar_v Multiplier for input point Q
/// @return X Resulting x-coordinate of the computed point
function mulmuladd(uint256 Q0, uint256 Q1, uint256 scalar_u, uint256 scalar_v) internal returns (uint256 X) {

Check warning on line 24 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase

Check warning on line 24 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase

Check warning on line 24 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase

Check warning on line 24 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase

Check warning on line 24 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase
uint256 zz;
uint256 zzz;
uint256 Y;

Check warning on line 27 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase
uint256 index = 255;
uint256[6] memory T;

Check warning on line 29 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase
uint256 H0;

Check warning on line 30 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase
uint256 H1;

Check warning on line 31 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Variable name must be in mixedCase

unchecked {
if (scalar_u == 0 && scalar_v == 0) return 0;
Expand All @@ -36,7 +36,7 @@
// will not work if Q=P, obvious forbidden private key
(H0, H1) = ECDSA.affAdd(gx, gy, Q0, Q1);

assembly {

Check warning on line 39 in src/ECDSA256r1.sol

View workflow job for this annotation

GitHub Actions / lint 

Avoid to use inline assembly. It is acceptable only in rare cases
for { let T4 := add(shl(1, and(shr(index, scalar_v), 1)), and(shr(index, scalar_u), 1)) } eq(T4, 0) {
index := sub(index, 1)
T4 := add(shl(1, and(shr(index, scalar_v), 1)), and(shr(index, scalar_u), 1))
Expand Down Expand Up @@ -131,15 +131,17 @@
T3 := mulmod(X, T2, p)
// W=UV
T1 := mulmod(T1, T2, p)
y2 := addmod(X, zz, p)
let TT1 := addmod(X, sub(p, zz), p)
// X-ZZ)(X+ZZ)
y2 := mulmod(y2, TT1, p)
// M

//(X-ZZ)(X+ZZ) -- xPlusZZ/xMinusZZ needed for stack management
let xPlusZZ := addmod(X, sub(p, zz), p)
let xMinusZZ := addmod(X, zz, p)
y2 := mulmod(xMinusZZ, xPlusZZ, p)

//M=3*(X-ZZ)(X+ZZ)
T4 := mulmod(3, y2, p)

// zzz3=W*zzz1
zzz := mulmod(TT1, zzz, p)
zzz := mulmod(T1, zzz, p)
// zz3=V*ZZ1, V free
zz := mulmod(T2, zz, p)

Expand Down
20 changes: 20 additions & 0 deletions test/unit/ecdsa256r1.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,26 @@ contract Ecdsa256r1Test is PRBTest {
uint256 _4P_res3 = implementation.mulmuladd(nQ[0], nQ[1], 2, 1);

assertEq(_4P_res1, _4P_res3);

// edge case from nlordell
(uint256 niordellX, uint256 niordellY, uint256 niordellU, uint256 niordellV) = (
0xe2534a3532d08fbba02dde659ee62bd0031fe2db785596ef509302446b030852, //x
0x1f0ea8a4b39cc339e62011a02579d289b103693d0cf11ffaa3bd3dc0e7b12739, //y
0xd13800358b760290af0671ee67368e9702a7145d1b9a0024b0b61ffe7bce9214, //u
0x344e000d62dd80a42bc19c7b99cda3a5c0a9c51746e680092c2d87ff9ef3af6f //v
);
assertEq(
0xcfcfa95b195904fd97b548d9e3cd2e023e06b4f10a87c645c7d4f74a0e206bad,
implementation.mulmuladd(niordellX, niordellY, niordellU, niordellV)
);

// edge case for Shamir
(uint256 shamirK, uint256 shamirX,) = (
0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc63254f, //k
0x7CF27B188D034F7E8A52380304B51AC3C08969E277F21B35A60B48FC47669978, // x
0xF888AAEE24712FC0D6C26539608BCF244582521AC3167DD661FB4862DD878C2E // y
);
assertEq(shamirX, implementation.mulmuladd(0, 0, shamirK, 0));
}

function test_MulMullAddMultipleBy0Fail_ReportSkip(uint256 q0, uint256 q1) public {
Expand Down
Loading