-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
R4R - Finish new docs v1 #5186
R4R - Finish new docs v1 #5186
Conversation
Codecov Report
@@ Coverage Diff @@
## master-docs #5186 +/- ##
===============================================
- Coverage 54.85% 54.83% -0.02%
===============================================
Files 305 305
Lines 18494 18494
===============================================
- Hits 10144 10142 -2
- Misses 7595 7597 +2
Partials 755 755 |
docs/basics/accounts-fees-gas.md
Outdated
} | ||
``` | ||
|
||
The default implementation of `Keybase` of the Cosmos SDK is [`dbKeybase`](https://github.com/cosmos/cosmos-sdk/blob/master/crypto/keys/keybase.go). A few notes on the `Keybase` methods as implemented in `dbKeybase`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the doc is moved in the future will it break these links?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well yes. We could use tags but the downside is that the link could become outdated instead of broken... Not sure one is better than the other (at least if it breaks we notice it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but how will it affect releaes, i thought we will have versioned docs and this would break on some versions and not on others, and backporting shouldn't be something we do for docs, maybe a script on docs build for releases that changes master
to a commit is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that might be a good solution, would that be possible @fadeev?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to substitute master
with something else when necessary, we specify a variable in config.js
and interpolate it into the URL {{version}}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should totally use permalinks here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to substitute master with something else when necessary, we specify a variable in config.js and interpolate it into the URL {{version}}.
this will only work for SDK links. Everything else should use a permalink
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have permalinks for tendermint stuff, do we? @marbar3778
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to worry about the repo disappearing case, there are bigger issues if that happens 🤣
docs/basics/accounts-fees-gas.md
Outdated
|
||
`Addresses` and `PubKey`s are both public information that identify actors in the application. There are 3 main types of `Addresses`/`PubKeys` available by default in the Cosmos SDK: | ||
|
||
- Addresses and Keys for **accounts**, which identify users (e.g. the sender of a `message`). They are derived using the **`secp256k1`** curve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe with #5006 this isnt entirely true any more, you could use other keys, right @AdityaSripal ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
antedecorator will have a separate docs PR afaik. This pr should be based on this branch @AdityaSripal
docs/core/events.md
Outdated
## Events | ||
|
||
`Event`s are implemented in the Cosmos SDK as an alias of the [ABCI `event` type](https://github.com/tendermint/tendermint/blob/master/abci/types/types.pb.go#L2661-L2667). They contain: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldnt use master branch here, these will be moving soon, can we use a commit/tag instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fadeev can we do the same thing with TM (i.e. variable interpolation via config.js
)
docs/basics/accounts-fees-gas.md
Outdated
A `Keybase` is an object that stores and manages accounts. In the Cosmos SDK, a `Keybase` implementation follows the [`Keybase` interface](https://github.com/cosmos/cosmos-sdk/blob/master/crypto/keys/types.go#L14-L60): | ||
|
||
```go | ||
type Keybase interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alessio will this change with the keyring PR?
See an [example of `ExportGenesis` from the nameservice tutorial](https://github.com/cosmos/sdk-application-tutorial/blob/master/x/nameservice/genesis.go#L46-L57). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use permalink
keeper.cdc.MustUnmarshalBinaryBare(bz, &object) | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo uvarint
docs/core/node.md
Outdated
|
||
```go | ||
tmNode, err := node.NewNode( | ||
cfg, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
padding is off
docs/core/node.md
Outdated
```go | ||
if err := tmNode.Start(); err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
padding is off
docs/core/node.md
Outdated
- First, a [`codec`](./encoding.md) is instanciated for the application. | ||
- Then, the [`config`](https://github.com/cosmos/cosmos-sdk/blob/master/types/config.go) is retrieved and config parameters are set. This mainly involves setting the bech32 prefixes for [addresses and pubkeys](../basics/accounts-fees-gas.md#addresses-and-pubkeys). | ||
- Using [cobra](https://github.com/spf13/cobra), the root command of the full-node client is created. After that, all the custom commands of the application are added using the `AddCommand()` method of `rootCmd`. | ||
- Add default server commands to `rootCmd` using the `server.AddCommands(ctx, cdc, rootCmd, newApp, exportAppStateAndTMValidators)` method. These commands are separated from the ones added above since they are standard and defined at SDK level. They should be shared by all SDK-based application. They include the most important command: the [`start` command](#start-command). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add default server commands to `rootCmd` using the `server.AddCommands(ctx, cdc, rootCmd, newApp, exportAppStateAndTMValidators)` method. These commands are separated from the ones added above since they are standard and defined at SDK level. They should be shared by all SDK-based application. They include the most important command: the [`start` command](#start-command). | |
- Add default server commands to `rootCmd` using the `server.AddCommands(ctx, cdc, rootCmd, newApp, exportAppStateAndTMValidators)` method. These commands are separated from the ones added above since they are standard and defined at SDK level. They should be shared by all SDK-based applications. They include the most important command: the [`start` command](#start-command). |
docs/core/store.md
Outdated
|
||
## Introduction to SDK Stores | ||
|
||
The Cosmos SDK comes with a large set of stores to persist the state of applications. By default, the main store of SDK applications is a multistore, i.e. a store of stores. Developers can add any number of key-value stores to the multistore, depending on their application needs. The multistore exists to support the modularity of the Cosmos SDK, as it lets each module declare and manage their own subset of the state. Key-value stores in the multistore can be accessed with a specific `key`, which is typically held in the [`keeper`](../building-modules/keeper.md) of the module that declared the store. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should mention that a StoreKey is really a capability key to access the store
docs/core/store.md
Outdated
Each Cosmos SDK application holds a multistore at its root to persist its state. The multistore is a store of `KVStores` that follows the `Multistore` interface: | ||
|
||
```go | ||
type MultiStore interface { //nolint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type MultiStore interface { //nolint | |
type MultiStore interface { |
docs/basics/accounts-fees-gas.md
Outdated
} | ||
``` | ||
|
||
The default implementation of `Keybase` of the Cosmos SDK is [`dbKeybase`](https://github.com/cosmos/cosmos-sdk/blob/master/crypto/keys/keybase.go). A few notes on the `Keybase` methods as implemented in `dbKeybase`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to substitute master with something else when necessary, we specify a variable in config.js and interpolate it into the URL {{version}}.
this will only work for SDK links. Everything else should use a permalink
docs/core/node.md
Outdated
appd start | ||
``` | ||
|
||
As a reminder, the full-node is composed of three conceptual layers: the networking layer, the consensus layer and the application layer. The first two are generally bundled together in an entity called the consensus engine, while the third is the state-machine defined with the help of the Cosmos SDK. Currently, the Cosmos SDK uses Tendermint as the default consensus engine, meaning the start command is implemented to boot up a Tendermint node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a reminder, the full-node is composed of three conceptual layers: the networking layer, the consensus layer and the application layer. The first two are generally bundled together in an entity called the consensus engine, while the third is the state-machine defined with the help of the Cosmos SDK. Currently, the Cosmos SDK uses Tendermint as the default consensus engine, meaning the start command is implemented to boot up a Tendermint node. | |
As a reminder, the full-node is composed of three conceptual layers: the networking layer, the consensus layer and the application layer. The first two are generally bundled together in `Tendermint Core` while the third is the state-machine defined with the help of the Cosmos SDK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"an entity call the consensus engine" is vaue, might want to specify Tendermint Core
docs/core/node.md
Outdated
|
||
As a reminder, the full-node is composed of three conceptual layers: the networking layer, the consensus layer and the application layer. The first two are generally bundled together in an entity called the consensus engine, while the third is the state-machine defined with the help of the Cosmos SDK. Currently, the Cosmos SDK uses Tendermint as the default consensus engine, meaning the start command is implemented to boot up a Tendermint node. | ||
|
||
The flow of the `start` command is pretty straightforward. First, it retrieves the `config` from the `context` in order to open the `db`. This `db` contains the latest known state of the application (empty if the application is started from the first time). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the db is an instance of IAVL yes? maybe specify that? https://github.com/tendermint/iavl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nah it's a leveldb
, I'll specify
a few minor changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Accounts Fees and Gas" is a heavily overloaded file. Why not split it into two? "Adresses and Accounts" & "Fees and Gas"
@fedekunze @hschoenburg integrated your comments, please approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at this point this pr is blocked on getting a ci process to replace master with commit hashes, or has this already happened?
docs/building-modules/handler.md
Outdated
Events: ctx.EventManager().Events(), | ||
} | ||
``` | ||
|
||
For a deeper look at `handler`s, see this [example implementation of a `handler` function](https://github.com/cosmos/sdk-application-tutorial/blob/master/x/nameservice/handler.go) from the nameservice tutorial. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this link will be broken in the coming days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job noticing that, I missed a few links. Added permalinks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be done for the sdk links as well? just thinking in terms of versioning
@marbar3778 if you mean CI process of https://allinbits.slack.com/archives/GDDLSDFPT/p1571509059077500 |
Added permalinks, and no the CI process should not block this PR as denis said. If nothing else please approve 🙏 |
ah I didn't mean that @fadeev meant a script to replace I think this script would have to be run on releases only though |
This PR contains the last core docs we need for "new docs v1"
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.
Wrote tests
Updated relevant documentation (
docs/
)Added a relevant changelog entry to the
Unreleased
section inCHANGELOG.md
Re-reviewed
Files changed
in the github PR explorerFor Admin Use: