Repository: NethermindEth/juno
Pull request #138 (2022-04-18 08:25:32)

refactor: update crypto
## Changes

Update the `keccak.Digest250` function to return a field element instead
of a byte array as specified in the documentation¹.

¹ https://docs.starknet.io/docs/Hashing/hash-functions#starknet-keccak

## Types of changes

- [x] Refactoring (no functional changes, no api changes)


## Testing
**Requires testing**

- [ ] Yes
- [x] No

codecov[bot]: # [Codecov](https://codecov.io/gh/NethermindEth/juno/pull/138?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) Report
> Merging [#138](https://codecov.io/gh/NethermindEth/juno/pull/138?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) (dc7d6ff) into [main](https://codecov.io/gh/NethermindEth/juno/commit/66ad185d85ff135566467f21590e9a79f833f2e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) (66ad185) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff            @@
##              main      #138   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1091      1095    +4     
=========================================
+ Hits          1091      1095    +4     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `100.00% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/NethermindEth/juno/pull/138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) | Coverage Δ | |
|---|---|---|
| [pkg/crypto/keccak/keccak.go](https://codecov.io/gh/NethermindEth/juno/pull/138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#diff-cGtnL2NyeXB0by9rZWNjYWsva2VjY2FrLmdv) | `100.00% <100.00%> (ø)` | |
| [pkg/crypto/pedersen/pedersen.go](https://codecov.io/gh/NethermindEth/juno/pull/138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#diff-cGtnL2NyeXB0by9wZWRlcnNlbi9wZWRlcnNlbi5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/rpc/server.go](https://codecov.io/gh/NethermindEth/juno/pull/138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#diff-cGtnL3JwYy9zZXJ2ZXIuZ28=) | `100.00% <0.00%> (ø)` | |
| [pkg/crypto/weierstrass/weierstrass.go](https://codecov.io/gh/NethermindEth/juno/pull/138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#diff-cGtnL2NyeXB0by93ZWllcnN0cmFzcy93ZWllcnN0cmFzcy5nbw==) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/NethermindEth/juno/pull/138?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/NethermindEth/juno/pull/138?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth). Last update [66ad185...dc7d6ff](https://codecov.io/gh/NethermindEth/juno/pull/138?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth).


Patch:
diff --git a/pkg/crypto/keccak/keccak.go b/pkg/crypto/keccak/keccak.go
index 64d41392..97a8c18b 100644
--- a/pkg/crypto/keccak/keccak.go
+++ b/pkg/crypto/keccak/keccak.go
@@ -3,19 +3,26 @@
 // For a detailed specification see: https://keccak.team.
 package keccak
 
-import "golang.org/x/crypto/sha3"
+import (
+	"math/big"
 
-// Digest250 returns the 256-bit Keccak hash with the last 6 bits set to
-// zero, as a 32 byte array. This is done so that the output fits inside
-// of a single felt.
-func Digest250(data []byte) []byte {
-	dgst := Digest256(data)
-	dgst[0] >>= 6 // Clear the last 6 bits.
-	return dgst
+	"golang.org/x/crypto/sha3"
+)
+
+// Digest250 returns the 256-bit Keccak hash of the data with the first
+// 6 bits set to zero. Note that signature differs from standard Go
+// hash functions in that the return type is a *big.Int meant to
+// represent a field element, 𝔽ₚ. See the [hash function] section of the
+// StarkNet documentation for details.
+//
+// [hash function]: https://docs.starknet.io/docs/Hashing/hash-functions#starknet-keccak
+func Digest250(data []byte) *big.Int {
+	digest := Digest256(data)
+	digest[0] >>= 6 // Clear the last 6 bits.
+	return new(big.Int).SetBytes(digest)
 }
 
-// Digest256 returns the 256-bit Keccak hash of the data as a slice of
-// 32 bytes.
+// Digest256 returns the 256-bit Keccak hash of the data.
 func Digest256(data []byte) []byte {
 	h := sha3.NewLegacyKeccak256()
 	h.Write(data)
diff --git a/pkg/crypto/pedersen/pedersen.go b/pkg/crypto/pedersen/pedersen.go
index 5f5a5d73..2a6d0fd4 100644
--- a/pkg/crypto/pedersen/pedersen.go
+++ b/pkg/crypto/pedersen/pedersen.go
@@ -27,7 +27,6 @@ var (
 	curve weierstrass.Curve
 	// zero is a *big.Int that represents the constant 0.
 	zero *big.Int
-
 	// ErrInvalid indicates an input value that is not a field element
 	// with p = 2²⁵¹ + 17·2¹⁹² + 1.
 	ErrInvalid = errors.New("invalid argument")
@@ -45,14 +44,17 @@ func init() {
 	zero = big.NewInt(0)
 }
 
-// NOTE: The upper bound on the number of `*big.Int`s that can hashed in
-// a single call (≤ 2) is not enforced for now.
-
 // Digest returns a field element that is the result of hashing an input
-// (a, b) ∈ 𝔽²ₚ where p = 2²⁵¹ + 17·2¹⁹² + 1.
+// (a, b) ∈ 𝔽²ₚ where p = 2²⁵¹ + 17·2¹⁹² + 1. This function will panic
+// if len(data) > 2 and return an error if (a, b) ∉ 𝔽²ₚ or the point at
+// infinity.
 func Digest(data ...*big.Int) (*big.Int, error) {
+	n := len(data)
+	if n > 2 {
+		panic("attempted to hash more than 2 field elements")
+	}
 	// Make a defensive copy of the input data.
-	elements := make([]*big.Int, len(data))
+	elements := make([]*big.Int, n)
 	for i, e := range data {
 		elements[i] = new(big.Int).Set(e)
 	}
@@ -61,7 +63,7 @@ func Digest(data ...*big.Int) (*big.Int, error) {
 		if !(x.Cmp(zero) != -1 && x.Cmp(curve.Params().P) == -1) {
 			// notest
 			// x is not in the range 0 < x < 2²⁵¹ + 17·2¹⁹² + 1.
-			return zero, ErrInvalid
+			return nil, ErrInvalid
 		}
 
 		for j := 0; j < 252; j++ {
@@ -69,10 +71,10 @@ func Digest(data ...*big.Int) (*big.Int, error) {
 			if pt1.x.Cmp(pt2.x) == 0 {
 				// notest
 				// Input cannot be hashed.
-				return zero, ErrInvalid
+				return nil, ErrInvalid
 			}
-			cp := new(big.Int).Set(x) // Copy because *big.Int.And mutates.
-			if cp.And(cp, big.NewInt(1)).Cmp(big.NewInt(0)) != 0 {
+			copy := new(big.Int).Set(x) // Copy because *big.Int.And mutates.
+			if copy.And(copy, big.NewInt(1)).Cmp(big.NewInt(0)) != 0 {
 				x1, x2 := curve.Add(pt1.x, pt1.y, pt2.x, pt2.y)
 				pt1 = point{x1, x2}
 			}


Test patch:
diff --git a/pkg/crypto/keccak/keccak_test.go b/pkg/crypto/keccak/keccak_test.go
index dc3787cf..6368d0e4 100644
--- a/pkg/crypto/keccak/keccak_test.go
+++ b/pkg/crypto/keccak/keccak_test.go
@@ -1,7 +1,6 @@
 package keccak
 
 import (
-	"encoding/hex"
 	"fmt"
 	"math/big"
 	"testing"
@@ -26,17 +25,17 @@ func BenchmarkDigest256(b *testing.B) {
 // ExampleDigest showcases the use of the Digest250 function in the
 // autogenerated documentation.
 func ExampleDigest250() {
-	dgst := Digest250([]byte("abc"))
-	fmt.Printf("0x%x\n", dgst)
+	digest := Digest250([]byte("abc"))
+	fmt.Printf("0x%x\n", digest)
 	// Output:
-	// 0x0103657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45
+	// 0x103657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45
 }
 
 // ExampleDigest256 showcases the use of the Digest250 function in the
 // autogenerated documentation.
 func ExampleDigest256() {
-	dgst := Digest256([]byte("abc"))
-	fmt.Printf("0x%x\n", dgst)
+	digest := Digest256([]byte("abc"))
+	fmt.Printf("0x%x\n", digest)
 	// Output:
 	// 0x4e03657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45
 }
@@ -47,8 +46,8 @@ func TestDigest250(t *testing.T) {
 	var tests = [...]struct {
 		input, want string
 	}{
-		{"", "03d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"},
-		{"abc", "0103657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45"},
+		{"", "3d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"},
+		{"abc", "103657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45"},
 	}
 	for _, test := range tests {
 		got := fmt.Sprintf("%x", Digest250([]byte(test.input)))
@@ -61,16 +60,12 @@ func TestDigest250(t *testing.T) {
 // TestDigest250Size checks that the hexadecimal representation of the
 // output of Digest250 is less than 2^250.
 func TestDigest250Size(t *testing.T) {
-	max := new(big.Int) // 2^250.
-	max.SetString("400000000000000000000000000000000000000000000000000000000000000", 16)
+	max, _ := new(big.Int).SetString("400000000000000000000000000000000000000000000000000000000000000", 16)
 	inputs := []string{"", "abc"}
 	for _, input := range inputs {
-		got := hex.EncodeToString(Digest250([]byte(input)))
-		i := new(big.Int)
-		i.SetString(got, 16)
-		// ^ (i < max).
-		if i.Cmp(max) != -1 {
-			t.Errorf("Digest250(%q) = 0x%x > 2^250 - 1", input, i)
+		got := Digest250([]byte(input))
+		if got.Cmp(max) != -1 {
+			t.Errorf("Digest250(%q) = 0x%x > 2^250 - 1", input, got)
 		}
 	}
 }
diff --git a/pkg/crypto/pedersen/pedersen_test.go b/pkg/crypto/pedersen/pedersen_test.go
index a84f87fd..dbcbced4 100644
--- a/pkg/crypto/pedersen/pedersen_test.go
+++ b/pkg/crypto/pedersen/pedersen_test.go
@@ -26,13 +26,13 @@ func BenchmarkDigest(b *testing.B) {
 func ExampleDigest() {
 	a, _ := new(big.Int).SetString("3d937c035c878245caf64531a5756109c53068da139362728feb561405371cb", 16)
 	b, _ := new(big.Int).SetString("208a0a10250e382e1e4bbe2880906c2791bf6275695e02fbbc6aeff9cd8b31a", 16)
-	dgst, err := Digest(a, b)
+	digest, err := Digest(a, b)
 	if err != nil {
 		panic(err)
 	}
-	fmt.Printf("0x%x\n", dgst)
+	fmt.Printf("%x\n", digest)
 	// Output:
-	// 0x30e480bed5fe53fa909cc0f8c4d99b8f9f2c016be4c41e13a4848797979c662
+	// 30e480bed5fe53fa909cc0f8c4d99b8f9f2c016be4c41e13a4848797979c662
 }
 
 // TestDigest does a basic test of the Pedersen hash function where the
@@ -60,7 +60,7 @@ func TestDigest(t *testing.T) {
 		want, _ := new(big.Int).SetString(test.want, 16)
 		got, _ := Digest(a, b)
 		if got.Cmp(want) != 0 {
-			t.Errorf("Digest(0x%x, 0x%x) = 0x%x, want 0x%x", a, b, got, want)
+			t.Errorf("Digest(%x, %x) = %x, want %x", a, b, got, want)
 		}
 	}
 }
@@ -111,7 +111,7 @@ func TestArrayDigest(t *testing.T) {
 		want, _ := new(big.Int).SetString(test.want, 16)
 		got, _ := ArrayDigest(data...)
 		if got.Cmp(want) != 0 {
-			t.Errorf("DigestArray(%v) = %v, want %v", data, got, want)
+			t.Errorf("ArrayDigest(%x) = %x, want %x", data, got, want)
 		}
 	}
 }