chore(ecocredit/core): keeper method audit (#1160)

* refactor(ecocredit/core): remove embedded cancel credits to its own type * refactor(ecocredit/core): improve error for invalid batch denom in cancel credits * perf(ecocredit/core): cleanup create batch method * Update x/ecocredit/server/core/create_batch.go Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com> * chore: better error for not found classId * fix: add better errors for ORM operations with user provided input * fix(ecocredit/simulation): check for not found error substr * fix: revert to utc Co-authored-by: tyler <tylergoodman@Tylers-MacBook-Pro.local> Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com> Co-authored-by: technicallyty <48813565+tytech3@users.noreply.github.com>
regen-network · Jun 13, 2022 · 17dda24 · 17dda24
1 parent 8e1914f
commit 17dda24
Show file tree

Hide file tree

Showing 30 changed files with 3,332 additions and 4,118 deletions.
diff --git a/api/regen/ecocredit/v1/tx.pulsar.go b/api/regen/ecocredit/v1/tx.pulsar.go
diff --git a/api/regen/ecocredit/v1/types.pulsar.go b/api/regen/ecocredit/v1/types.pulsar.go
diff --git a/proto/regen/ecocredit/v1/tx.proto b/proto/regen/ecocredit/v1/tx.proto
@@ -322,23 +322,11 @@ message MsgCancel {
   string owner = 1;
 
   // credits are the credits being cancelled.
-  repeated CancelCredits credits = 2;
+  repeated Credits credits = 2;
 
   // reason is any arbitrary string that specifies the reason for cancelling
   // credits.
   string reason = 3;
-
-  // CancelCredits specifies a batch and the number of credits being cancelled.
-  message CancelCredits {
-
-    // batch_denom is the unique identifier of the credit batch.
-    string batch_denom = 1;
-
-    // amount is the number of credits being cancelled.
-    // Decimal values are acceptable within the precision returned by
-    // Query/Precision.
-    string amount = 2;
-  }
 }
 
 // MsgCancelResponse is the Msg/Cancel response type.
@@ -444,19 +432,7 @@ message MsgBridge {
   string owner = 4;
 
   // credits are the credits being cancelled.
-  repeated CancelCredits credits = 5;
-
-  // CancelCredits specifies a batch and the number of credits being cancelled.
-  message CancelCredits {
-
-    // batch_denom is the unique identifier of the credit batch.
-    string batch_denom = 1;
-
-    // amount is the number of credits being cancelled.
-    // Decimal values are acceptable within the precision returned by
-    // Query/Precision.
-    string amount = 2;
-  }
+  repeated Credits credits = 5;
 }
 
 // MsgBridgeResponse is the Msg/MsgBridge response type.

diff --git a/proto/regen/ecocredit/v1/types.proto b/proto/regen/ecocredit/v1/types.proto
@@ -59,3 +59,12 @@ message CreditTypeProposal {
   // passes.
   CreditType credit_type = 3;
 }
+
+// Credits represents a simple structure for credits.
+message Credits {
+  // batch_denom is the denom of the credit batch
+  string batch_denom = 1;
+
+  // amount is the amount of credits
+  string amount = 2;
+}
diff --git a/x/ecocredit/client/util.go b/x/ecocredit/client/util.go
@@ -57,15 +57,15 @@ var (
 	reCredits   = regexp.MustCompile(fmt.Sprintf(`^(%s) (%s)$`, reCreditAmt, core.RegexBatchDenom))
 )
 
-func parseCancelCreditsList(creditsListStr string) ([]*core.MsgCancel_CancelCredits, error) {
+func parseCancelCreditsList(creditsListStr string) ([]*core.Credits, error) {
 	creditsList, err := parseCreditsList(creditsListStr)
 	if err != nil {
 		return nil, err
 	}
 
-	cancelCreditsList := make([]*core.MsgCancel_CancelCredits, len(creditsList))
+	cancelCreditsList := make([]*core.Credits, len(creditsList))
 	for i, credits := range creditsList {
-		cancelCreditsList[i] = &core.MsgCancel_CancelCredits{
+		cancelCreditsList[i] = &core.Credits{
 			BatchDenom: credits.batchDenom,
 			Amount:     credits.amount,
 		}

diff --git a/x/ecocredit/core/msg_bridge_test.go b/x/ecocredit/core/msg_bridge_test.go
@@ -3,8 +3,9 @@ package core
 import (
 	"testing"
 
-	"github.com/regen-network/regen-ledger/types/testutil"
 	"github.com/stretchr/testify/require"
+
+	"github.com/regen-network/regen-ledger/types/testutil"
 )
 
 func TestMsgBridge(t *testing.T) {
@@ -21,7 +22,7 @@ func TestMsgBridge(t *testing.T) {
 		"valid msg": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -35,7 +36,7 @@ func TestMsgBridge(t *testing.T) {
 		},
 		"invalid msg without owner": {
 			src: MsgBridge{
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -50,7 +51,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg with wrong owner address": {
 			src: MsgBridge{
 				Owner: "wrong owner",
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -68,7 +69,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg without Credits.BatchDenom": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						Amount: "10",
 					},
@@ -79,7 +80,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg without Credits.Amount": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 					},
@@ -90,7 +91,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg with wrong Credits.Amount": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "abc",
@@ -102,7 +103,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg without bridge target": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -116,7 +117,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg without bridge contract": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -130,7 +131,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid msg without bridge recipient address": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -144,7 +145,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid bridge recipient address": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -159,7 +160,7 @@ func TestMsgBridge(t *testing.T) {
 		"invalid bridge target": {
 			src: MsgBridge{
 				Owner: addr1,
-				Credits: []*MsgBridge_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",

diff --git a/x/ecocredit/core/msg_cancel_test.go b/x/ecocredit/core/msg_cancel_test.go
@@ -3,8 +3,9 @@ package core
 import (
 	"testing"
 
-	"github.com/regen-network/regen-ledger/types/testutil"
 	"github.com/stretchr/testify/require"
+
+	"github.com/regen-network/regen-ledger/types/testutil"
 )
 
 func TestMsgCancel(t *testing.T) {
@@ -19,7 +20,7 @@ func TestMsgCancel(t *testing.T) {
 		"valid msg": {
 			src: MsgCancel{
 				Owner: addr1,
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -31,7 +32,7 @@ func TestMsgCancel(t *testing.T) {
 		},
 		"invalid msg without holder": {
 			src: MsgCancel{
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -43,7 +44,7 @@ func TestMsgCancel(t *testing.T) {
 		"invalid msg with wrong holder address": {
 			src: MsgCancel{
 				Owner: "wrong owner",
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "10",
@@ -61,7 +62,7 @@ func TestMsgCancel(t *testing.T) {
 		"invalid msg without Credits.BatchDenom": {
 			src: MsgCancel{
 				Owner: addr1,
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						Amount: "10",
 					},
@@ -72,7 +73,7 @@ func TestMsgCancel(t *testing.T) {
 		"invalid msg without Credits.Amount": {
 			src: MsgCancel{
 				Owner: addr1,
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 					},
@@ -83,7 +84,7 @@ func TestMsgCancel(t *testing.T) {
 		"invalid msg with wrong Credits.Amount": {
 			src: MsgCancel{
 				Owner: addr1,
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "abc",
@@ -95,7 +96,7 @@ func TestMsgCancel(t *testing.T) {
 		"invalid msg reason is required": {
 			src: MsgCancel{
 				Owner: addr1,
-				Credits: []*MsgCancel_CancelCredits{
+				Credits: []*Credits{
 					{
 						BatchDenom: batchDenom,
 						Amount:     "1",