From 291d966fe93eba49a669e3e397122f3256550f21 Mon Sep 17 00:00:00 2001
From: Emmanuel T Odeke <emmanuel@orijtech.com>
Date: Mon, 7 Dec 2020 12:57:15 -0800
Subject: [PATCH] x/ibc/core/23-commitment/types: fix MerkleProof.Empty
 comparisons (#8092)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* x/ibc/core/23-commitment/types: fix MerkleProof.Empty comparisons

Fixes invalid pointer creation, reduces on unnecessary allocations
inside MerkleProof.Empty, and changes the method to a pointer; the
invalid pointer creation was symptomatic of broad use of values.

With this change we have improvements whose benchmarks produce:

```shell
name     old time/op    new time/op    delta
Empty-8     311ns ± 5%     232ns ± 5%  -25.49%  (p=0.000 n=20+19)

name     old alloc/op   new alloc/op   delta
Empty-8     56.0B ± 0%      8.0B ± 0%  -85.71%  (p=0.000 n=20+20)

name     old allocs/op  new allocs/op  delta
Empty-8      3.00 ± 0%      1.00 ± 0%  -66.67%  (p=0.000 n=20+20)
```

Fixes #8091

* Move Empty godoc to right place + add comments for blank*
---
 x/ibc/core/23-commitment/types/bench_test.go | 15 +++++++++++++++
 x/ibc/core/23-commitment/types/merkle.go     |  9 +++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)
 create mode 100644 x/ibc/core/23-commitment/types/bench_test.go

diff --git a/x/ibc/core/23-commitment/types/bench_test.go b/x/ibc/core/23-commitment/types/bench_test.go
new file mode 100644
index 000000000..83794fc6f
--- /dev/null
+++ b/x/ibc/core/23-commitment/types/bench_test.go
@@ -0,0 +1,15 @@
+package types
+
+import (
+	"testing"
+)
+
+func BenchmarkMerkleProofEmpty(b *testing.B) {
+	b.ReportAllocs()
+	var mk MerkleProof
+	for i := 0; i < b.N; i++ {
+		if !mk.Empty() {
+			b.Fatal("supposed to be empty")
+		}
+	}
+}
diff --git a/x/ibc/core/23-commitment/types/merkle.go b/x/ibc/core/23-commitment/types/merkle.go
index fe7d5c302..e90fccc34 100644
--- a/x/ibc/core/23-commitment/types/merkle.go
+++ b/x/ibc/core/23-commitment/types/merkle.go
@@ -269,9 +269,14 @@ func verifyChainedMembershipProof(root []byte, specs []*ics23.ProofSpec, proofs
 	return nil
 }
 
+// blankMerkleProof and blankProofOps will be used to compare against their zero values,
+// and are declared as globals to avoid having to unnecessarily re-allocate on every comparison.
+var blankMerkleProof = &MerkleProof{}
+var blankProofOps = &tmcrypto.ProofOps{}
+
 // Empty returns true if the root is empty
-func (proof MerkleProof) Empty() bool {
-	return proto.Equal(&proof, nil) || proto.Equal(&proof, &MerkleProof{}) || proto.Equal(&proof, &tmcrypto.ProofOps{})
+func (proof *MerkleProof) Empty() bool {
+	return proof == nil || proto.Equal(proof, blankMerkleProof) || proto.Equal(proof, blankProofOps)
 }
 
 // ValidateBasic checks if the proof is empty.