refactor(ecocredit): update genesis import & export to use v1 api #977

aleem1314 · 2022-04-04T10:01:43Z

Description

Closes: #976

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...

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)

x/ecocredit/module/module_test.go

…m/regen-network/regen-ledger into aleem/ecocredit-genesis-migration

x/ecocredit/genesis.go

x/ecocredit/client/testsuite/tx.go

x/ecocredit/server/server_test.go

x/ecocredit/simulation/genesis.go

x/ecocredit/genesis.go

aaronc · 2022-04-08T21:54:26Z

If we need to check invariants in validation, we should just import to an in memory orm db and run the invariants. Does that make sense?

…m/regen-network/regen-ledger into aleem/ecocredit-genesis-migration

codecov · 2022-04-11T09:03:27Z

Codecov Report

Merging #977 (1c66a72) into master (0a35fad) will decrease coverage by 10.90%.
The diff coverage is 60.36%.

❗ Current head 1c66a72 differs from pull request most recent head 5f50723. Consider uploading reports for the commit 5f50723 to get more accurate results

@@             Coverage Diff             @@
##           master     #977       +/-   ##
===========================================
- Coverage   60.14%   49.24%   -10.91%     
===========================================
  Files         221      221               
  Lines       23493    23334      -159     
===========================================
- Hits        14130    11490     -2640     
- Misses       8043    10595     +2552     
+ Partials     1320     1249       -71

Flag	Coverage Δ
experimental-codecov	`49.13% <60.36%> (-10.90%)`	⬇️
stable-codecov	`42.02% <60.36%> (-11.88%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

technicallyty

this is great work 👏🏻 just a few comments/suggestions

x/ecocredit/core/genesis.go

ryanchristo

This is looking great. Very nice work. A few questions and comments.

x/ecocredit/core/genesis.go

x/ecocredit/core/genesis_test.go

Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com>

technicallyty

didnt catch these in my last review, but should be good to go after this

x/ecocredit/core/genesis.go

technicallyty · 2022-04-12T17:22:24Z

x/ecocredit/core/genesis_test.go

+				"address":"gydQIvR2RUi0N1RJnmgOLVSkcd4=",
+				"batch_id":"1",
+				"tradable":"90.003",
+				"retired":"9.997"


we could add escrowed balance here as a test case

Looks like this might have been overlooked. We should include a test case with a positive escrowed amount.

It also looks like you are missing a BatchBalance validation.

Also CreditType and BatchSupply validation.

Looks like this might have been overlooked. We should include a test case with a positive escrowed amount.

713955d

Also CreditType and BatchSupply validation.

Added validation for CreditType. Batch balance & supply validation is already covered by ValidateGenesis.

ryanchristo

Looking good. One minor suggestion, message field validation as you proposed, and changes to include escrowed.

x/ecocredit/core/genesis_test.go

…m/regen-network/regen-ledger into aleem/ecocredit-genesis-migration

…into aleem/ecocredit-genesis-migration

ryanchristo

Updates look great. The proto message validation is a nice add. A few more comments.

x/ecocredit/core/genesis.go

ryanchristo · 2022-04-13T20:35:58Z

x/ecocredit/core/genesis_test.go

+				"address":"gydQIvR2RUi0N1RJnmgOLVSkcd4=",
+				"batch_id":"1",
+				"tradable":"90.003",
+				"retired":"9.997"


Looks like this might have been overlooked. We should include a test case with a positive escrowed amount.

ryanchristo · 2022-04-13T20:41:28Z

x/ecocredit/core/genesis_test.go

+				"address":"gydQIvR2RUi0N1RJnmgOLVSkcd4=",
+				"batch_id":"1",
+				"tradable":"90.003",
+				"retired":"9.997"


It also looks like you are missing a BatchBalance validation.

ryanchristo · 2022-04-13T20:47:29Z

x/ecocredit/core/genesis_test.go

+				"address":"gydQIvR2RUi0N1RJnmgOLVSkcd4=",
+				"batch_id":"1",
+				"tradable":"90.003",
+				"retired":"9.997"


Also CreditType and BatchSupply validation.

Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com>

ryanchristo

This looks great. Very nice work.

technicallyty

lgtm

wip: migrate genesis

86009b7

aleem1314 changed the title ~~wip: migrate genesis~~ refactor(ecocredit): update genesis import & export to use v1 api Apr 4, 2022

aleem1314 mentioned this pull request Apr 6, 2022

refactor(ecocredit): update core simulations to use v1 api #928

Merged

19 tasks

aleem1314 added 3 commits April 8, 2022 16:34

fix: tests

1f86fa4

cleanup genesis sims

a3ff33b

Merge branch 'master' into aleem/ecocredit-genesis-migration

aa1b135

aleem1314 commented Apr 8, 2022

View reviewed changes

x/ecocredit/module/module_test.go Outdated Show resolved Hide resolved

aleem1314 and others added 4 commits April 8, 2022 16:49

Update x/ecocredit/module/module_test.go

2561ded

chore: disable ecocredit tests

235d9b9

remove legacy genesis

ead088b

Merge branch 'aleem/ecocredit-genesis-migration' of https://github.co…

c572105

…m/regen-network/regen-ledger into aleem/ecocredit-genesis-migration

aleem1314 marked this pull request as ready for review April 8, 2022 12:13

aleem1314 commented Apr 8, 2022

View reviewed changes

x/ecocredit/genesis.go Show resolved Hide resolved

aleem1314 requested review from ryanchristo, technicallyty and aaronc April 8, 2022 12:34

ryanchristo reviewed Apr 8, 2022

View reviewed changes

x/ecocredit/client/testsuite/tx.go Outdated Show resolved Hide resolved

x/ecocredit/server/server_test.go Show resolved Hide resolved

x/ecocredit/simulation/genesis.go Show resolved Hide resolved

x/ecocredit/genesis.go Show resolved Hide resolved

ryanchristo requested a review from robert-zaremba April 8, 2022 18:29

ryanchristo mentioned this pull request Apr 8, 2022

refactor(x/ecocredit): migrate ecocredit server to submodules #995

Merged

19 tasks

aleem1314 added 2 commits April 9, 2022 17:43

wip: use memdb for genesis validation

6460cb1

wip: add tests

840549d

aleem1314 marked this pull request as draft April 10, 2022 15:20

aleem1314 and others added 4 commits April 10, 2022 22:03

migrate core sims to v1

d0d46ea

update genesis tests

e8e6ad9

chore: add todo for commented code

0b89b4d

Merge branch 'master' into aleem/ecocredit-genesis-migration

016f0c6

aleem1314 requested a review from ryanchristo April 11, 2022 07:40

aleem1314 marked this pull request as ready for review April 11, 2022 07:40

remove test

44948ec

Merge branch 'aleem/ecocredit-genesis-migration' of https://github.co…

e31a6aa

…m/regen-network/regen-ledger into aleem/ecocredit-genesis-migration

chore: cleanup

eb63b2c

technicallyty reviewed Apr 11, 2022

View reviewed changes

ryanchristo reviewed Apr 11, 2022

View reviewed changes

aleem1314 and others added 3 commits April 12, 2022 10:11

Update x/ecocredit/core/genesis.go

7fe26b6

Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com>

chore: review changes

90d0872

Merge branch 'master' into aleem/ecocredit-genesis-migration

dd2d7f0

aleem1314 requested review from ryanchristo and technicallyty April 12, 2022 16:32

technicallyty reviewed Apr 12, 2022

View reviewed changes

ryanchristo reviewed Apr 12, 2022

View reviewed changes

x/ecocredit/core/genesis_test.go Show resolved Hide resolved

aleem1314 added 3 commits April 13, 2022 14:36

chore: add core state types validation

c6e0839

Merge branch 'aleem/ecocredit-genesis-migration' of https://github.co…

c3e3d17

…m/regen-network/regen-ledger into aleem/ecocredit-genesis-migration

Merge branch 'master' of https://github.com/regen-network/regen-ledger …

a583f4e

…into aleem/ecocredit-genesis-migration

aleem1314 requested review from ryanchristo and technicallyty April 13, 2022 12:04

ryanchristo reviewed Apr 13, 2022

View reviewed changes

aleem1314 and others added 2 commits April 14, 2022 16:42

Update x/ecocredit/core/genesis.go

5180f2b

Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com>

chore: review changes

1c66a72

ryanchristo approved these changes Apr 14, 2022

View reviewed changes

technicallyty approved these changes Apr 15, 2022

View reviewed changes

Merge branch 'master' into aleem/ecocredit-genesis-migration

5f50723

ryanchristo enabled auto-merge (squash) April 15, 2022 03:57

ryanchristo merged commit b21763e into master Apr 15, 2022

ryanchristo deleted the aleem/ecocredit-genesis-migration branch April 15, 2022 04:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ecocredit): update genesis import & export to use v1 api #977

refactor(ecocredit): update genesis import & export to use v1 api #977

aleem1314 commented Apr 4, 2022 •

edited by zube bot

Loading

aaronc commented Apr 8, 2022

codecov bot commented Apr 11, 2022 •

edited

Loading

technicallyty left a comment

ryanchristo left a comment

technicallyty left a comment

technicallyty Apr 12, 2022

ryanchristo Apr 13, 2022

ryanchristo Apr 13, 2022

ryanchristo Apr 13, 2022

aleem1314 Apr 14, 2022 •

edited

Loading

ryanchristo left a comment •

edited

Loading

ryanchristo left a comment

ryanchristo Apr 13, 2022

ryanchristo Apr 13, 2022

ryanchristo Apr 13, 2022

ryanchristo left a comment

technicallyty left a comment

refactor(ecocredit): update genesis import & export to use v1 api #977

refactor(ecocredit): update genesis import & export to use v1 api #977

Conversation

aleem1314 commented Apr 4, 2022 • edited by zube bot Loading

Description

Author Checklist

Reviewers Checklist

aaronc commented Apr 8, 2022

codecov bot commented Apr 11, 2022 • edited Loading

Codecov Report

technicallyty left a comment

Choose a reason for hiding this comment

ryanchristo left a comment

Choose a reason for hiding this comment

technicallyty left a comment

Choose a reason for hiding this comment

technicallyty Apr 12, 2022

Choose a reason for hiding this comment

ryanchristo Apr 13, 2022

Choose a reason for hiding this comment

ryanchristo Apr 13, 2022

Choose a reason for hiding this comment

ryanchristo Apr 13, 2022

Choose a reason for hiding this comment

aleem1314 Apr 14, 2022 • edited Loading

Choose a reason for hiding this comment

ryanchristo left a comment • edited Loading

Choose a reason for hiding this comment

ryanchristo left a comment

Choose a reason for hiding this comment

ryanchristo Apr 13, 2022

Choose a reason for hiding this comment

ryanchristo Apr 13, 2022

Choose a reason for hiding this comment

ryanchristo Apr 13, 2022

Choose a reason for hiding this comment

ryanchristo left a comment

Choose a reason for hiding this comment

technicallyty left a comment

Choose a reason for hiding this comment

aleem1314 commented Apr 4, 2022 •

edited by zube bot

Loading

codecov bot commented Apr 11, 2022 •

edited

Loading

aleem1314 Apr 14, 2022 •

edited

Loading

ryanchristo left a comment •

edited

Loading