diff --git a/btcec/pubkey.go b/btcec/pubkey.go index e05092d1f..be6f85097 100644 --- a/btcec/pubkey.go +++ b/btcec/pubkey.go @@ -153,6 +153,14 @@ func (p *PublicKey) SerializeHybrid() []byte { return paddedAppend(32, b, p.Y.Bytes()) } +// IsEqual compares this PublicKey instance to the one passed, returning true if +// both PublicKeys are equivalent. A PublicKey is equivalent to another, if they +// both have the same X and Y coordinate. +func (p *PublicKey) IsEqual(otherPubKey *PublicKey) bool { + return p.X.Cmp(otherPubKey.X) == 0 && + p.Y.Cmp(otherPubKey.Y) == 0 +} + // paddedAppend appends the src byte slice to dst, returning the new slice. // If the length of the source is smaller than the passed size, leading zero // bytes are appended to the dst slice before appending src. diff --git a/btcec/pubkey_test.go b/btcec/pubkey_test.go index a499655d5..16c6f2572 100644 --- a/btcec/pubkey_test.go +++ b/btcec/pubkey_test.go @@ -247,3 +247,39 @@ func TestPubKeys(t *testing.T) { } } } + +func TestPublicKeyIsEqual(t *testing.T) { + pubKey1, err := btcec.ParsePubKey( + []byte{0x03, 0x26, 0x89, 0xc7, 0xc2, 0xda, 0xb1, 0x33, + 0x09, 0xfb, 0x14, 0x3e, 0x0e, 0x8f, 0xe3, 0x96, 0x34, + 0x25, 0x21, 0x88, 0x7e, 0x97, 0x66, 0x90, 0xb6, 0xb4, + 0x7f, 0x5b, 0x2a, 0x4b, 0x7d, 0x44, 0x8e, + }, + btcec.S256(), + ) + if err != nil { + t.Fatalf("failed to parse raw bytes for pubKey1: %v", err) + } + + pubKey2, err := btcec.ParsePubKey( + []byte{0x02, 0xce, 0x0b, 0x14, 0xfb, 0x84, 0x2b, 0x1b, + 0xa5, 0x49, 0xfd, 0xd6, 0x75, 0xc9, 0x80, 0x75, 0xf1, + 0x2e, 0x9c, 0x51, 0x0f, 0x8e, 0xf5, 0x2b, 0xd0, 0x21, + 0xa9, 0xa1, 0xf4, 0x80, 0x9d, 0x3b, 0x4d, + }, + btcec.S256(), + ) + if err != nil { + t.Fatalf("failed to parse raw bytes for pubKey2: %v", err) + } + + if !pubKey1.IsEqual(pubKey1) { + t.Fatalf("value of IsEqual is incorrect, %v is "+ + "equal to %v", pubKey1, pubKey1) + } + + if pubKey1.IsEqual(pubKey2) { + t.Fatalf("value of IsEqual is incorrect, %v is not "+ + "equal to %v", pubKey1, pubKey2) + } +} diff --git a/btcec/signature.go b/btcec/signature.go index 126f8944b..ab5a345c4 100644 --- a/btcec/signature.go +++ b/btcec/signature.go @@ -82,6 +82,14 @@ func (sig *Signature) Verify(hash []byte, pubKey *PublicKey) bool { return ecdsa.Verify(pubKey.ToECDSA(), hash, sig.R, sig.S) } +// IsEqual compares this Signature instance to the one passed, returning true +// if both Signatures are equivalent. A signature is equivalent to another, if +// they both have the same scalar value for R and S. +func (sig *Signature) IsEqual(otherSig *Signature) bool { + return sig.R.Cmp(otherSig.R) == 0 && + sig.S.Cmp(otherSig.S) == 0 +} + func parseSig(sigStr []byte, curve elliptic.Curve, der bool) (*Signature, error) { // Originally this code used encoding/asn1 in order to parse the // signature, but a number of problems were found with this approach. diff --git a/btcec/signature_test.go b/btcec/signature_test.go index 739a0fba1..c13bbe0f9 100644 --- a/btcec/signature_test.go +++ b/btcec/signature_test.go @@ -588,3 +588,24 @@ func TestRFC6979(t *testing.T) { } } } + +func TestSignatureIsEqual(t *testing.T) { + sig1 := &btcec.Signature{ + R: fromHex("0082235e21a2300022738dabb8e1bbd9d19cfb1e7ab8c30a23b0afbb8d178abcf3"), + S: fromHex("24bf68e256c534ddfaf966bf908deb944305596f7bdcc38d69acad7f9c868724"), + } + sig2 := &btcec.Signature{ + R: fromHex("4e45e16932b8af514961a1d3a1a25fdf3f4f7732e9d624c6c61548ab5fb8cd41"), + S: fromHex("181522ec8eca07de4860a4acdd12909d831cc56cbbac4622082221a8768d1d09"), + } + + if !sig1.IsEqual(sig1) { + t.Fatalf("value of IsEqual is incorrect, %v is "+ + "equal to %v", sig1, sig1) + } + + if sig1.IsEqual(sig2) { + t.Fatalf("value of IsEqual is incorrect, %v is not "+ + "equal to %v", sig1, sig2) + } +} diff --git a/config.go b/config.go index 3c26bc140..0e00caa0d 100644 --- a/config.go +++ b/config.go @@ -46,7 +46,7 @@ const ( defaultGenerate = false defaultMaxOrphanTransactions = 1000 defaultMaxOrphanTxSize = 5000 - defaultSigCacheMaxSize = 50000 + defaultSigCacheMaxSize = 100000 defaultTxIndex = false defaultAddrIndex = false ) diff --git a/txscript/reference_test.go b/txscript/reference_test.go index e180993cf..9abab7d0f 100644 --- a/txscript/reference_test.go +++ b/txscript/reference_test.go @@ -194,6 +194,7 @@ func TestScriptInvalidTests(t *testing.T) { return } sigCache := NewSigCache(10) + sigCacheToggle := []bool{true, false} for _, useSigCache := range sigCacheToggle { for i, test := range tests { @@ -258,7 +259,9 @@ func TestScriptValidTests(t *testing.T) { err) return } + sigCache := NewSigCache(10) + sigCacheToggle := []bool{true, false} for _, useSigCache := range sigCacheToggle { for i, test := range tests { diff --git a/txscript/sigcache.go b/txscript/sigcache.go index a49a1b792..028a267cc 100644 --- a/txscript/sigcache.go +++ b/txscript/sigcache.go @@ -5,20 +5,21 @@ package txscript import ( - "bytes" - "crypto/rand" "sync" "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/wire" ) -// sigInfo represents an entry in the SigCache. Entries in the sigcache are a -// 3-tuple: (sigHash, sig, pubKey). -type sigInfo struct { - sigHash wire.ShaHash - sig string - pubKey string +// sigCacheEntry represents an entry in the SigCache. Entries within the +// SigCache are keyed according to the sigHash of the signature. In the +// scenario of a cache-hit (according to the sigHash), an additional comparison +// of the signature, and public key will be executed in order to ensure a complete +// match. In the occasion that two sigHashes collide, the newer sigHash will +// simply overwrite the existing entry. +type sigCacheEntry struct { + sig *btcec.Signature + pubKey *btcec.PublicKey } // SigCache implements an ECDSA signature verification cache with a randomized @@ -33,7 +34,7 @@ type sigInfo struct { // if they've already been seen and verified within the mempool. type SigCache struct { sync.RWMutex - validSigs map[sigInfo]struct{} + validSigs map[wire.ShaHash]sigCacheEntry maxEntries uint } @@ -43,7 +44,10 @@ type SigCache struct { // to make room for new entries that would cause the number of entries in the // cache to exceed the max. func NewSigCache(maxEntries uint) *SigCache { - return &SigCache{validSigs: make(map[sigInfo]struct{}), maxEntries: maxEntries} + return &SigCache{ + validSigs: make(map[wire.ShaHash]sigCacheEntry, maxEntries), + maxEntries: maxEntries, + } } // Exists returns true if an existing entry of 'sig' over 'sigHash' for public @@ -52,13 +56,14 @@ func NewSigCache(maxEntries uint) *SigCache { // NOTE: This function is safe for concurrent access. Readers won't be blocked // unless there exists a writer, adding an entry to the SigCache. func (s *SigCache) Exists(sigHash wire.ShaHash, sig *btcec.Signature, pubKey *btcec.PublicKey) bool { - info := sigInfo{sigHash, string(sig.Serialize()), - string(pubKey.SerializeCompressed())} - s.RLock() - _, ok := s.validSigs[info] - s.RUnlock() - return ok + defer s.RUnlock() + + if entry, ok := s.validSigs[sigHash]; ok { + return entry.pubKey.IsEqual(pubKey) && entry.sig.IsEqual(sig) + } + + return false } // Add adds an entry for a signature over 'sigHash' under public key 'pubKey' @@ -79,35 +84,19 @@ func (s *SigCache) Add(sigHash wire.ShaHash, sig *btcec.Signature, pubKey *btcec // If adding this new entry will put us over the max number of allowed // entries, then evict an entry. if uint(len(s.validSigs)+1) > s.maxEntries { - // Generate a cryptographically random hash. - randHashBytes := make([]byte, wire.HashSize) - _, err := rand.Read(randHashBytes) - if err != nil { - // Failure to read a random hash results in the proposed - // entry not being added to the cache since we are - // unable to evict any existing entries. - return - } - - // Try to find the first entry that is greater than the random - // hash. Use the first entry (which is already pseudo random due - // to Go's range statement over maps) as a fall back if none of - // the hashes in the rejected transactions pool are larger than - // the random hash. - var foundEntry sigInfo + // Remove a random entry from the map relaying on the random + // starting point of Go's map iteration. It's worth noting that + // the random iteration starting point is not 100% guaranteed + // by the spec, however most Go compilers support it. + // Ultimately, the iteration order isn't important here because + // in order to manipulate which items are evicted, an adversary + // would need to be able to execute preimage attacks on the + // hashing function in order to start eviction at a specific + // entry. for sigEntry := range s.validSigs { - if foundEntry.sig == "" { - foundEntry = sigEntry - } - if bytes.Compare(sigEntry.sigHash.Bytes(), randHashBytes) > 0 { - foundEntry = sigEntry - break - } + delete(s.validSigs, sigEntry) + break } - delete(s.validSigs, foundEntry) } - - info := sigInfo{sigHash, string(sig.Serialize()), - string(pubKey.SerializeCompressed())} - s.validSigs[info] = struct{}{} + s.validSigs[sigHash] = sigCacheEntry{sig, pubKey} }