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

Update SPEC-SPEC & documentation for x/{bank,evidence} #7404

Merged
merged 8 commits into from Sep 28, 2020
Merged

Conversation

amaury1093
Copy link
Contributor

@amaury1093 amaury1093 commented Sep 28, 2020

Description

ref: #6953

For x/bank and x/evidence:

  • read through the docs to make sure they're still up to date
  • replace deprecated go interfaces/structs with their newer versions
  • use protobuf message instead of go structs, where possible
  • use a single README.md instead of separate spec files.

Rendered:


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.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@amaury1093 amaury1093 changed the title Update documentation for x/bank Update documentation for x/{bank,evidence} Sep 28, 2020
@alexanderbez
Copy link
Contributor

THANK YOU <3

@amaury1093 amaury1093 marked this pull request as ready for review September 28, 2020 13:57
@amaury1093
Copy link
Contributor Author

I'll put R4R, and will tackle other modules separetely.

@fedekunze
Copy link
Collaborator

fedekunze commented Sep 28, 2020

This document should follow the SPEC-SPEC for consistency between the documents (I'm fine with changing it as long as it's consistent and documented 👍 )

@amaury1093
Copy link
Contributor Author

Thanks for the link, I wasn't aware of SPEC-SPEC. I'll update it to reflect this need then.

@alexanderbez
Copy link
Contributor

@fedekunze please see my comment here. I believe the format/layout needs to be updated.

@amaury1093
Copy link
Contributor Author

@fedekunze LMK if this updated SPEC-SPEC works for you.

@amaury1093 amaury1093 changed the title Update documentation for x/{bank,evidence} Update SPEC-SPEC & documentation for x/{bank,evidence} Sep 28, 2020
@fedekunze fedekunze added the spec label Sep 28, 2020
@fedekunze fedekunze merged commit e17dd4f into master Sep 28, 2020
@fedekunze fedekunze deleted the am-modules branch September 28, 2020 16:51
## Supply
## 02. Concepts

### Supply

The `supply` module:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@amaurymartiny Should this refer to the "bank" instaed of "supply" module, as supply is not a module anymore? Or maybe referring to as supply functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. These docs were added at the same time as merging x/supply into x/bank (#6010), so I thought they made sense. I'll replace with "supply functionality".

Edit: #7407


The supply module introduces a new type of `auth.Account` which can be used by
modules to allocate tokens and in special cases mint or burn tokens. At a base
The supply module introduces a new type of `auth.AccountI` interface, called `ModuleAccountI`, which can be used by
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly, there isn't a supply module anymore, so we should probably remove references to it as such.

Comment on lines -1 to -6
<!--
order: 0
title: Bank Overview
parent:
title: "bank"
-->
Copy link
Member

Choose a reason for hiding this comment

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

Please add this back, this is needed for the docs site. I commented on the merged commit, moving the comment here

Comment on lines -1 to -6
<!--
order: 0
title: Evidence Overview
parent:
title: "evidence"
-->
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@alexanderbez
Copy link
Contributor

@marbar3778 I suggested a single file. Can the docs website not just use a single file? I suppose if the docs website can do it, then perhaps Atlas can too. Sorry @amaurymartiny maybe we revert the idea of using a single doc.

@amaury1093
Copy link
Contributor Author

I'm fine with both, I only touched 4 modules, so can revert. Would just like to have a final decision on what people prefer.

I personally prefer having separate .md files, to avoid having md files with 1k+ lines.

Can the docs website not just use a single file?

Yeah that's fine, it'll be one long page per module.

then perhaps Atlas can too

I thought that was the main decisive factor:

  • if Atlas can only read one single README.md, then let's continue like this.
  • if you think Atlas could potentially take into account multiple md files, I'll revert.

@alexanderbez lmk how we should move forward.

@tac0turtle
Copy link
Member

tac0turtle commented Sep 29, 2020

@marbar3778 I suggested a single file. Can the docs website not just use a single file? I suppose if the docs website can do it, then perhaps Atlas can too. Sorry @amaurymartiny maybe we revert the idea of using a single doc.

I did not mean to say revert the single file change, just meant add back the removal of the header. The docs use this for titles of the files.

I personally prefer having separate .md files, to avoid having md files with 1k+ lines.

I also prefer this. If the move is single file I am also fine with this as well.

@alexanderbez
Copy link
Contributor

Sorry, let's go back to multiple files with a main README.md that navigates to each separate file. Sorry for the trouble @amaurymartiny. I'm sure I can figure out how to handle this in Atlas.

amaury1093 added a commit that referenced this pull request Sep 29, 2020
mergify bot added a commit that referenced this pull request Oct 16, 2020
)

* Revert some changes from #7404

* Update x/slashing

* Address review comments

* Small tweak

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
clevinson pushed a commit that referenced this pull request Oct 19, 2020
)

* Revert some changes from #7404

* Update x/slashing

* Address review comments

* Small tweak

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
clevinson added a commit that referenced this pull request Oct 19, 2020
* Update Encoding Doc for 0.40 (#7430)

* Remove deprecated docs and add first guidelines to protobuf migration

* Add conventions

* Reorder and add more FAQ doc

* Update guidelines for pb msg definitions

* Use commit hash in github links

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* docs: Update "Basics" section (#7416)

* Prettier

* docs: Update "Basics" section

* appcli -> appd

* Better wording

* Fix to appCodec

* Add gRPC mention

* Add grpc

* Reference simapp code

* Update docs/basics/accounts.md

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>

* Add section about gRPC query services

* Optional LegacyQuerierHandler

* Clearer docs

* Update docs/basics/app-anatomy.md

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>

* Update docs/basics/app-anatomy.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Address comments

* Address comments

* Update docs/basics/accounts.md

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Fix misbehaviour handling for solo machine (#7515)

* add timestamp to SignatureAndData

Add timestamp field to signature and data. Add ValidateBasic check for timestamp. Add ValidateBasic test. Update misbehaviour handler to use supplied timestamp.

* fix typo

* add timestamp check

* fix lint

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* update solo machine specs (#7512)

* update solo machine specs

* update concepts

* self review fixes

* Apply suggestions from code review

* add note on upgarding solo machines

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Corrected 'unsafe-reset-all' help text (#7504)

* Merge PR #7415: Return empty store tree on non-existing version

* tendermint: update sdk to rc5 (#7527)

* update sdk to rc5

* Update baseapp/params.go

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

* Bump github.com/spf13/cobra from 1.0.0 to 1.1.0 (#7553)

Bumps [github.com/spf13/cobra](https://github.com/spf13/cobra) from 1.0.0 to 1.1.0.
- [Release notes](https://github.com/spf13/cobra/releases)
- [Changelog](https://github.com/spf13/cobra/blob/master/CHANGELOG.md)
- [Commits](spf13/cobra@v1.0.0...v1.1.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* remove test_utils.go in tm client (#7522)

* remove test_utils from tm client

* fix build

* fix lint?

* fix lint?

* apply @fedekunze review suggestion

* add tests as per @alessio suggestion

* fix typo

* ibc: cleanup channel types test (#7521)

* Update Building Modules documentation (#7473)

* Update module-manager.md

* Update modules #messages doc

* Fix typo

* Update message implementation example link

* Update messages-and-queries.md#queries doc

* Update querier.md

* Update handler.md

* Update links in beginblock-endblock.md

* Update keeper.md

* Update invariants.md

* Update genesis.md

* Update module-interfaces.md#transaction-commands

* Update module-interfaces.md#query-commands

* Update module-interfaces.md#flags

* Update module-interfaces.md#rest

* Update structure.md

* Update module-interfaces.md#grpc

* Update errors.md

* Update module-interfaces.md#grpc-gateway-rest

* Add more info on swagger

* Address comments

* Fix go.sum

* Fix app-anatomy.md

* Update docs/building-modules/module-interfaces.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update docs/building-modules/querier.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update docs/building-modules/querier.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update docs/building-modules/module-interfaces.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Update docs/building-modules/module-interfaces.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* Address part of review comments

* Update old ref of RegisterQueryService

* Add example code for Manager

* Update queriers.md to query-services.md and refs

* Add new query-services.md

* Revert "Update old ref of RegisterQueryService"

This reverts commit 1ea1ea8.

* Update keeper.md

* Update handler.md

* Update handler.md

* Update messages-and-queries.md

* Update docs/basics/app-anatomy.md

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>

* Update docs/building-modules/intro.md

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>

* Fix typo

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>

* client: add GetAccount and GetAccountWithHeight to AccountRetriever (#7558)

* client: add GetAccount and GetAccountWithHeight to AccountRetriever

* update ADR

* address comments from review

* Add the option of emitting amino encoded json from the CLI (#7221)

* Add the option of emitting amino encoded json

* Update AMINO JSON serialization with ConvertTxToStdTx

* Make the Amino flag more self documenting by serializing the BroadcastRequest type instead of StdTx

* Handle amino encoding error

* Update x/auth/client/cli/tx_multisign.go

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* Update x/auth/client/cli/tx_sign.go

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* Apply suggestions from code review

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* Fix go format

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* ibc: update solo machine client command (#7579)

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* docs: Revert SPEC-SPEC and update x/{auth,bank,evidence,slashing} (#7407)

* Revert some changes from #7404

* Update x/slashing

* Address review comments

* Small tweak

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* remove id in localhost (#7577)

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Christopher Goes <cwgoes@pluranimity.org>

* update changelog

* Update CHANGELOG.md

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

* Fix solomachine cmds (#7581)

* fix solo machine cli cmds

* polish

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* simapp: use tmjson on InitChainer (#7514)

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* [IAVL]: Bump to v0.15.0-rc4 (#7549)

* iavl 0.15.0-rc4 version bump

* update comments on error modes for store.LoadStore

* update CONFIO_URL in makefile

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Handle nil *Any in UnpackAny and add panic handler for tx decoding (#7594)

* Handle nil any in UnpackAny

* Add test

* Add flag back

* Update runTx signature

* Update Simulate signature

* Update calls to Simulate

* Add txEncoder in baseapp

* Fix TestTxWithoutPublicKey

* Wrap errors

* Use amino in baseapp tests

* Add txEncoder arg to Check & Deliver

* Fix gas in test

* Fix remaining base app tests

* Rename to amionTxEncoder

* Update codec/types/interface_registry.go

Co-authored-by: Aaron Craelius <aaron@regen.network>

* golangci-lint fix

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
Co-authored-by: Aaron Craelius <aaron@regen.network>
Co-authored-by: Cory Levinson <cjlevinson@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Neeraj Murarka <njmurarka@users.noreply.github.com>
Co-authored-by: Riccardo Montagnin <riccardo.montagnin@gmail.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Zaki Manian <zaki@manian.org>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Christopher Goes <cwgoes@pluranimity.org>
Co-authored-by: Aaron Craelius <aaron@regen.network>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants