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

transfer (total escrow): add documentation and migration docs #3509

Merged
merged 4 commits into from
Apr 27, 2023

Conversation

crodriguezvega
Copy link
Contributor

@crodriguezvega crodriguezvega commented Apr 23, 2023

Description

ref: #3368

Commit Message / Changelog Entry

N/A

see the guidelines for commit messages. (view raw markdown for examples)


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md).
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/).
  • Added relevant godoc comments.
  • Provide a commit message to be used for the changelog entry in the PR description for review.
  • Re-reviewed Files changed in the Github PR explorer.
  • Review Codecov Report in the comment section below once CI passes.

docs/migrations/v7-to-v7_1.md Outdated Show resolved Hide resolved
Copy link
Contributor

@DimitrisJim DimitrisJim left a comment

Choose a reason for hiding this comment

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

small typo nits

docs/apps/transfer/client.md Show resolved Hide resolved
docs/apps/transfer/client.md Show resolved Hide resolved
@colin-axner
Copy link
Contributor

merging into feature branch, followup prs always welcome

@colin-axner colin-axner merged commit a448960 into feat/total-escrow-state-entry Apr 27, 2023
@colin-axner colin-axner deleted the carlos/total-escrow-docs branch April 27, 2023 16:37
crodriguezvega added a commit that referenced this pull request Jun 1, 2023
* transfer (total escrow): add documentation and migration docs (#3509)

* add docs for total escrow feature

* revert change

* fix tag

* Update docs/migrations/v7-to-v7_1.md

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* transfer (total escrow): some more review comments (#3519)

* some more review comments

* Rename pathAndEscrowAMount to pathAndEscrowAmount

---------

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

* transfer (total escrow): add invariant for total escrow (#3506)

* add invariant for total escrow

* address review comment

* refactor: simplify logic for asserting invariant

* fix: use safeAdd instead of append

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

---------

Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* imp: do not store total escrow when amount is zero (#3585)

* do not store 0 escrow amout

* adapt success test

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add comments

---------

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add feature release for total escrow state entry to conditionally query the endpoint in e2e (#3605)

* fix total escrow cli documentation

* Apply suggestions from code review

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* address review comments

* docs: adr 011 total escrow state entry (#3641)

* docs: add adr 011 for total escrow state entry

* indentation

* code formatting

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Charly <charly@interchain.io>

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>
mergify bot pushed a commit that referenced this pull request Jun 1, 2023
* transfer (total escrow): add documentation and migration docs (#3509)

* add docs for total escrow feature

* revert change

* fix tag

* Update docs/migrations/v7-to-v7_1.md

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* transfer (total escrow): some more review comments (#3519)

* some more review comments

* Rename pathAndEscrowAMount to pathAndEscrowAmount

---------

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

* transfer (total escrow): add invariant for total escrow (#3506)

* add invariant for total escrow

* address review comment

* refactor: simplify logic for asserting invariant

* fix: use safeAdd instead of append

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

---------

Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* imp: do not store total escrow when amount is zero (#3585)

* do not store 0 escrow amout

* adapt success test

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add comments

---------

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add feature release for total escrow state entry to conditionally query the endpoint in e2e (#3605)

* fix total escrow cli documentation

* Apply suggestions from code review

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* address review comments

* docs: adr 011 total escrow state entry (#3641)

* docs: add adr 011 for total escrow state entry

* indentation

* code formatting

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Charly <charly@interchain.io>

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>
(cherry picked from commit 6f628d9)

# Conflicts:
#	docs/architecture/README.md
#	docs/migrations/v7-to-v7_1.md
#	e2e/testconfig/testconfig.go
#	e2e/tests/transfer/base_test.go
crodriguezvega added a commit that referenced this pull request Jun 1, 2023
* imp: transfer total escrow follow ups (#3558)

* transfer (total escrow): add documentation and migration docs (#3509)

* add docs for total escrow feature

* revert change

* fix tag

* Update docs/migrations/v7-to-v7_1.md

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* transfer (total escrow): some more review comments (#3519)

* some more review comments

* Rename pathAndEscrowAMount to pathAndEscrowAmount

---------

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

* transfer (total escrow): add invariant for total escrow (#3506)

* add invariant for total escrow

* address review comment

* refactor: simplify logic for asserting invariant

* fix: use safeAdd instead of append

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

---------

Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* imp: do not store total escrow when amount is zero (#3585)

* do not store 0 escrow amout

* adapt success test

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add comments

---------

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add feature release for total escrow state entry to conditionally query the endpoint in e2e (#3605)

* fix total escrow cli documentation

* Apply suggestions from code review

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* address review comments

* docs: adr 011 total escrow state entry (#3641)

* docs: add adr 011 for total escrow state entry

* indentation

* code formatting

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Charly <charly@interchain.io>

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>
(cherry picked from commit 6f628d9)

# Conflicts:
#	docs/architecture/README.md
#	docs/migrations/v7-to-v7_1.md
#	e2e/testconfig/testconfig.go
#	e2e/tests/transfer/base_test.go

* fix conflicts

* remove dead links

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
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.

3 participants