Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(x/ecocredit/v3/client): add/update independent project CLI commands #2202

Closed
wants to merge 87 commits into from

Conversation

aaronc
Copy link
Member

@aaronc aaronc commented Apr 9, 2024

Description

Closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@aaronc
Copy link
Member Author

aaronc commented May 28, 2024

Merged this into #2167

@aaronc aaronc closed this May 28, 2024
Copy link
Contributor

@paul121 paul121 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working well! I didn't find any major state or logic issues while testing. Tests seem to cover this pretty well 👍

Just found various little things, mostly that a few of the CLI commands weren't quite wired up and could maybe be improved for consistency.

Simple fixes I can't suggest via github UI:

  • Need to add project enrollment query cmds to x/ecocredit/client/query.go QueryCmd():

            baseclient.QueryProjectEnrollmentCmd(),
            baseclient.QueryProjectEnrollmentsCmd(),
    
  • Error with the example batch.json in create-batch command help text. It is missing a class_id

  • Doesn't need to be fixed in this PR, but the ecocredit script in images/regen-sandbox/setup/ecocredit.sh needs some updating (some commands fail due to these changes)

The general cases I tested:

  • Creating projects independent vs direct, testing correct permissions to update project metadata, etc
  • Creating application, withdrawing application, testing correct permissions + good/bad project + class IDs, metadata
  • Updating enrollment status, testing correct permissions, correct status transitions, good/bad project + class IDs, metadata
  • Creating credit batch, testing correct permissions, good/bad project + class IDs
  • "Full flow": Create project, apply, enroll, issue credits, terminate enrollment, try to issue credits, re-enroll, issue credits

I think this covered using/testing the following CLI commands:

  • tx ecocredit:

    • create-class
    • create-project
    • update-project-metadata
    • create-or-update-application
    • withdraw-application
    • update-project-enrollment
    • create-batch
    • gen-batch-json
  • q ecocredit:

    • batches
    • class, classes
    • class-issuers
    • project, projects
    • project-enrollment
    • project-enrollments

Could improve error messages when proj + class enrollments do not exist/have an entry in the enrollment DB table:

  • withdraw-application
    • when the credit class or project ID does not exist gets generic error msg: raw_log: 'failed to execute message; message index: 0: not found'
    • when valid credit class and project ID, but no application/enrollment between the two, gets a better error msg: raw_log: 'failed to execute message; message index: 0: cannot withdraw non-existent application: invalid request'
  • create-batch
    • when credit class does not exist or when no enrollment for project + class: raw_log: 'failed to execute message; message index: 0: not found'
    • when project does not exist, better error msg: raw_log: 'failed to execute message; message index: 0: could not get project with id P005: not found: invalid request'

Project enrollment status - just an idea

  • There is no way for a credit class to signal that they have "received" an application and are beginning review, maybe give an expected timeframe, etc. I assume this could happen via enrollment metadata.
    • The credit class can only add metadata when setting the status to ACCEPTED or CHANGES_REQUESTED.
    • Would it make sense to add a project enrollment status for "PENDING" / "IN REVIEW" so there is an intermediary status?

Use: "create-project [jurisdiction] [metadata] [flags]",
Short: "Create a new project",
Long: `Create a new project. By default, this creates an unregistered project that is not enrolled in any credit
class. If the class-id flag is provided, the signer must be an issuer of the class and the project will be enrolled in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class. If the class-id flag is provided, the signer must be an issuer of the class and the project will be enrolled in
class. If the class flag is provided, the signer must be an issuer of the class and the project will be enrolled in

Comment on lines +39 to +40
baseclient.TxCreateOrUpdateApplicationCmd(),
baseclient.TxWithdrawApplicationCmd(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the update-project-enrollment cmd

Suggested change
baseclient.TxCreateOrUpdateApplicationCmd(),
baseclient.TxWithdrawApplicationCmd(),
baseclient.TxCreateOrUpdateApplicationCmd(),
baseclient.TxWithdrawApplicationCmd(),
baseclient.TxUpdateProjectEnrollmentCmd(),

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
}

func TxUpdateProjectEnrollment() *cobra.Command {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add Cmd suffix for consistency?

Suggested change
func TxUpdateProjectEnrollment() *cobra.Command {
func TxUpdateProjectEnrollmentCmd() *cobra.Command {

We explicitly include the project creation fee here so that the project creator acknowledges paying
the fee and is not surprised to learn that the they paid a fee without consent.
- class: the ID of the credit class. If this is provided, the signer must be an issuer of the class.
- reference-id: a reference ID for the project. Only valid if class-id is also provided.`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- reference-id: a reference ID for the project. Only valid if class-id is also provided.`,
- reference-id: a reference ID for the project. Only valid if class is also provided.`,

- class: the ID of the credit class. If this is provided, the signer must be an issuer of the class.
- reference-id: a reference ID for the project. Only valid if class-id is also provided.`,
Example: `regen tx ecocredit create-project "US-WA 98225" regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf
regen tx ecocredit create-project "US-WA 98225" regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf --class-id C01 --reference-id VCS-001`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
regen tx ecocredit create-project "US-WA 98225" regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf --class-id C01 --reference-id VCS-001`,
regen tx ecocredit create-project "US-WA 98225" regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf --class C01 --reference-id VCS-001`,

Short: "Generates JSON to represent a new credit batch for use with create-batch command",
Long: `Generates JSON to represent a new credit batch for use with create-batch command.

Parameters:

- issuer: the account address of the credit batch issuer
- project-id: the ID of the project
- class-id: the ID of the credit class to which the batch belongs and in which the project is enrolled
- issuance-count: the number of issuance items to generate
- metadata: any arbitrary metadata to attach to the credit batch
- start-date: the beginning of the period during which this credit batch was
quantified and verified (format: yyyy-mm-dd)
- end-date: the end of the period during which this credit batch was
quantified and verified (format: yyyy-mm-dd)`,
Example: "regen tx ecocredit gen-batch-json regen1elq7ys34gpkj3jyvqee0h6yk4h9wsfxmgqelsw C01-001 2 regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf 2020-01-01 2021-01-01",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Example: "regen tx ecocredit gen-batch-json regen1elq7ys34gpkj3jyvqee0h6yk4h9wsfxmgqelsw C01-001 2 regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf 2020-01-01 2021-01-01",
Example: "regen tx ecocredit gen-batch-json regen1elq7ys34gpkj3jyvqee0h6yk4h9wsfxmgqelsw P001 C01 2 regen:13toVgf5UjYBz6J29x28pLQyjKz5FpcW3f4bT5uRKGxGREWGKjEdXYG.rdf 2020-01-01 2021-01-01",

@@ -667,3 +667,66 @@ func QueryAllowedBridgeChainsCmd() *cobra.Command {
flags.AddPaginationFlagsToCmd(cmd, "allowed-bridge-chains")
return qflags(cmd)
}

func QueryProjectEnrollment() *cobra.Command {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consistency

Suggested change
func QueryProjectEnrollment() *cobra.Command {
func QueryProjectEnrollmentCmd() *cobra.Command {

return qflags(cmd)
}

func QueryProjectEnrollments() *cobra.Command {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consistency

Suggested change
func QueryProjectEnrollments() *cobra.Command {
func QueryProjectEnrollmentsCmd() *cobra.Command {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants