feat(x/ecocredit): unique reference id at message server level (#1228)

* feat(x/ecocredit): unique reference id at message server level

* add and update project tests

* add and update project tests

* clean up

* update bridge receive comments

* update bridge receive comments

* update bridge receive comments

* fix tests

* Update x/ecocredit/server/core/msg_create_project_test.go

* Apply suggestions from code review

* Update x/ecocredit/server/core/create_project.go

Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>

Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>

x/ecocredit/client/testsuite/tx.go

@@ -991,7 +991,7 @@ func (s *IntegrationTestSuite) TestUpdateProjectAdmin() {
 		ClassId:      s.classId,
 		Metadata:     "metadata",
 		Jurisdiction: "US-WA",
-		ReferenceId:  s.projectReferenceId,
+		ReferenceId:  "VCS-002",
 	})
 
 	// create new project in order to not interfere with other tests
@@ -1000,7 +1000,7 @@ func (s *IntegrationTestSuite) TestUpdateProjectAdmin() {
 		ClassId:      s.classId,
 		Metadata:     "metadata",
 		Jurisdiction: "US-WA",
-		ReferenceId:  s.projectReferenceId,
+		ReferenceId:  "VCS-003",
 	})
 
 	// create new project in order to not interfere with other tests
@@ -1009,7 +1009,7 @@ func (s *IntegrationTestSuite) TestUpdateProjectAdmin() {
 		ClassId:      s.classId,
 		Metadata:     "metadata",
 		Jurisdiction: "US-WA",
-		ReferenceId:  s.projectReferenceId,
+		ReferenceId:  "VCS-004",
 	})
 
 	testCases := []struct {

x/ecocredit/core/utils.go

@@ -153,6 +153,19 @@ func ValidateJurisdiction(jurisdiction string) error {
 	return nil
 }
 
+// GetClassIdFromProjectId returns the credit class ID in a project ID.
+func GetClassIdFromProjectId(projectId string) string {
+	var s strings.Builder
+	for _, r := range projectId {
+		if r != '-' {
+			s.WriteRune(r)
+			continue
+		}
+		break
+	}
+	return s.String()
+}
+
 // GetClassIdFromBatchDenom returns the credit class ID in a batch denom.
 func GetClassIdFromBatchDenom(denom string) string {
 	var s strings.Builder

x/ecocredit/core/utils_test.go

@@ -15,6 +15,7 @@ func TestUtils(t *testing.T) {
 	t.Run("TestInvalidProjectId", rapid.MakeCheck(testInvalidProjectId))
 	t.Run("TestFormatBatchDenom", rapid.MakeCheck(testFormatBatchDenom))
 	t.Run("TestInvalidBatchDenom", rapid.MakeCheck(testInvalidBatchDenom))
+	t.Run("TestGetClassIdFromProjectId", rapid.MakeCheck(testGetClassIdFromProjectId))
 	t.Run("TestGetClassIdFromBatchDenom", rapid.MakeCheck(testGetClassIdFromBatchDenom))
 	t.Run("GetCreditTypeAbbrevFromClassId", rapid.MakeCheck(testGetCreditTypeAbbrevFromClassId))
 }
@@ -79,6 +80,18 @@ func testInvalidBatchDenom(t *rapid.T) {
 	require.Error(t, ValidateBatchDenom(batchDenom))
 }
 
+func testGetClassIdFromProjectId(t *rapid.T) {
+	creditType := genCreditType.Draw(t, "creditType").(*CreditType)
+	classSeq := rapid.Uint64().Draw(t, "classSeq").(uint64)
+	projectSeq := rapid.Uint64().Draw(t, "projectSeq").(uint64)
+
+	classId := FormatClassId(creditType.Abbreviation, classSeq)
+	projectId := FormatProjectId(classId, projectSeq)
+
+	result := GetClassIdFromProjectId(projectId)
+	require.Equal(t, classId, result)
+}
+
 func testGetClassIdFromBatchDenom(t *rapid.T) {
 	creditType := genCreditType.Draw(t, "creditType").(*CreditType)
 	classSeq := rapid.Uint64().Draw(t, "classSeq").(uint64)

x/ecocredit/server/core/bridge_receive.go

@@ -173,9 +173,11 @@ func (k Keeper) getBatchFromBridgeReq(ctx context.Context, req *core.MsgBridgeRe
 	return nil, nil
 }
 
-// getProjectFromBridgeReq attempts to get a project from state given the request.
-// The first project seen in the iterator is returned.
-// If no projects are found, nil is returned for both values.
+// getProjectFromBridgeReq attempts to find a project with a matching reference id within the scope
+// of the credit class. No more than one project will be returned when we list the projects based on
+// class id and reference id because we enforce uniqueness on non-empty reference ids within the scope
+// of a credit class (and we do this at the message server level and not the ORM level because reference
+// id is optional when using Msg/CreateProject). If no project is found, nil is returned for both values.
 func (k Keeper) getProjectFromBridgeReq(ctx context.Context, req *core.MsgBridgeReceive_Project, classId string) (*api.Project, error) {
 	class, err := k.stateStore.ClassTable().GetById(ctx, classId)
 	if err != nil {
@@ -189,7 +191,9 @@ func (k Keeper) getProjectFromBridgeReq(ctx context.Context, req *core.MsgBridge
 		return nil, err
 	}
 
-	// we only want the first project that matches the reference ID, so we do not loop here.
+	// we only want the first project that matches the reference ID, so we do not loop here. We enforce
+	// uniqueness for a non-empty reference id at the message service level so as long as the reference
+	// id is not empty (verified in validate basic), no more than one project will ever be returned.
 	var project *api.Project
 	if it.Next() {
 		var err error
@@ -199,5 +203,6 @@ func (k Keeper) getProjectFromBridgeReq(ctx context.Context, req *core.MsgBridge
 		}
 	}
 	it.Close()
+
 	return project, nil
 }

x/ecocredit/server/core/create_project.go

@@ -35,6 +35,12 @@ func (k Keeper) CreateProject(ctx context.Context, req *core.MsgCreateProject) (
 		return nil, err
 	}
 
+	// check if non-empty reference id is unique within the scope of the credit class
+	err = k.verifyReferenceId(ctx, classInfo.Key, req.ReferenceId)
+	if err != nil {
+		return nil, err
+	}
+
 	if err = k.stateStore.ProjectTable().Insert(ctx, &api.Project{
 		Id:           projectID,
 		Admin:        adminAddress,
@@ -80,3 +86,28 @@ func (k Keeper) genProjectID(ctx context.Context, classKey uint64, classID strin
 
 	return core.FormatProjectId(classID, nextSeq), nil
 }
+
+// verifyReferenceId prevents multiple projects from having the same reference id within the
+// scope of a credit class. We verify this here at the message server level rather than at the
+// ORM level because reference id is optional and therefore multiple projects within the scope
+// of a credit class can have an empty reference id (see BridgeReceive for more information)
+func (k Keeper) verifyReferenceId(ctx context.Context, classKey uint64, referenceId string) error {
+	if referenceId == "" {
+		// reference id is optional so an empty reference id is valid
+		return nil
+	}
+
+	key := api.ProjectClassKeyReferenceIdIndexKey{}.WithClassKeyReferenceId(classKey, referenceId)
+	it, err := k.stateStore.ProjectTable().List(ctx, key)
+	if err != nil {
+		return err
+	}
+        defer it.Close()
+	if it.Next() {
+		return sdkerrors.ErrInvalidRequest.Wrapf(
+			"a project with reference id %s already exists within this credit class", referenceId,
+		)
+	}
+
+	return nil
+}

x/ecocredit/server/core/features/msg_create_project.feature

@@ -2,33 +2,34 @@ Feature: CreateProject
 
   Projects can be created:
   - when the creator is an approved credit class issuer
+  - when the non-empty reference id is unique within the scope of the credit class
   - ...
 
   Rule: A project id is always unique
 
     Scenario: multiple projects from the same credit class
-      Given a credit type exists with abbreviation "A"
-      And alice has created a credit class with credit type "A"
-      And alice has created a project with credit class id "A01"
-      When alice creates a project with credit class id "A01"
-      Then the project exists with project id "A01-002"
+      Given a credit type with abbreviation "A"
+      And a credit class with class id "A01" and issuer alice
+      And a project with project id "A01-001"
+      When alice attempts to create a project with class id "A01"
+      Then expect project with project id "A01-002"
 
     Scenario: multiple projects from different credit classes
-      Given a credit type exists with abbreviation "A"
-      And a credit type exists with abbreviation "B"
-      And alice has created a credit class with credit type "A"
-      And alice has created a credit class with credit type "B"
-      And alice has created a project with credit class id "A01"
-      And alice has created a project with credit class id "B01"
-      When alice creates a project with credit class id "B01"
-      Then the project exists with project id "B01-001"
+      Given a credit type with abbreviation "A"
+      And a credit type with abbreviation "B"
+      And a credit class with class id "A01" and issuer alice
+      And a credit class with class id "B01" and issuer alice
+      And a project with project id "A01-001"
+      And a project with project id "B01-001"
+      When alice attempts to create a project with class id "B01"
+      Then expect project with project id "B01-002"
 
     Scenario Outline: project id is formatted correctly
-      Given a credit type exists with abbreviation "A"
-      And alice has created a credit class with credit type "A"
-      And the project sequence for credit class "A01" is "<next_sequence>"
-      When alice creates a project with credit class id "A01"
-      Then the project exists with project id "<project_id>"
+      Given a credit type with abbreviation "A"
+      And a credit class with class id "A01" and issuer alice
+      And a project sequence "<next_sequence>" for credit class "A01"
+      When alice attempts to create a project with class id "A01"
+      Then expect project with project id "<project_id>"
 
       Examples:
         | next_sequence | project_id  |
@@ -38,3 +39,24 @@ Feature: CreateProject
         | 100           | A01-100     |
         | 1000          | A01-1000    |
         | 10000         | A01-10000   |
+
+  Rule: A non-empty reference id must be unique within the scope of a credit class
+
+    Background:
+      Given a credit type with abbreviation "C"
+      And a credit class with class id "C01" and issuer alice
+
+    Scenario: non-empty reference id is unique within credit class
+      Given a project with project id "C01-001" and reference id "VCS-001"
+      When alice attempts to create a project with class id "C01" and reference id "VCS-002"
+      Then expect no error
+
+    Scenario: empty reference id is allowed for multiple projects
+      Given a project with project id "C01-001" and reference id ""
+      When alice attempts to create a project with class id "C01" and reference id ""
+      Then expect no error
+
+    Scenario: non-empty reference id is not unique within credit class
+      Given a project with project id "C01-001" and reference id "VCS-001"
+      When alice attempts to create a project with class id "C01" and reference id "VCS-001"
+      Then expect the error "a project with reference id VCS-001 already exists within this credit class: invalid request"