docs: guidelines for ValidateBasic

docs/basics/tx-lifecycle.md

@@ -83,7 +83,15 @@ When `Tx` is received by the application from the underlying consensus engine (e
 
 ### ValidateBasic
 
-[`sdk.Msg`s](../core/transactions.md#messages) are extracted from `Tx`, and `ValidateBasic`, a method of the `sdk.Msg` interface implemented by the module developer, is run for each one. `ValidateBasic` should include basic **stateless** sanity checks. For example, if the message is to send coins from one address to another, `ValidateBasic` likely checks for nonempty addresses and a nonnegative coin amount, but does not require knowledge of state such as the account balance of an address.
+[`sdk.Msg`s](../core/transactions.md#messages) are extracted from `Tx`, and `ValidateBasic`, a method of the `sdk.Msg` interface implemented by the module developer, is run for each one. 
+This method is called by the `BaseApp` very early in the processing of the `message` (in both [`CheckTx`](../core/baseapp.md#checktx) and [`DeliverTx`](../core/baseapp.md#delivertx)), in order to discard obviously invalid messages.
+`ValidateBasic` can only include **stateless** checks, i.e. checks that do not require access to the state. 
+
+#### Guideline
+
+Gas won't be charged when `ValidateBasic` is executed. Hence, we should only do the most necessary sanity checks to enable middleware operations (eg: parsing the required signer accounts to validate a signature by a middleware). Other validation operation should be performed when [handling a message](../building-modules/msg-services#Validation) in a module Msg Server.
+
+For example, if the message is to send coins from one address to another, `ValidateBasic` likely checks for nonempty addresses and a nonnegative coin amount, but does not require knowledge of state such as the account balance of an address.
 
 ### AnteHandler
 

docs/building-modules/messages-and-queries.md

@@ -55,7 +55,7 @@ It extends `proto.Message` and contains the following methods:
 
 - `Route() string`: Name of the route for this message. Typically all `message`s in a module have the same route, which is most often the module's name.
 - `Type() string`: Type of the message, used primarly in [events](../core/events.md). This should return a message-specific `string`, typically the denomination of the message itself.
-- `ValidateBasic() error`: This method is called by `BaseApp` very early in the processing of the `message` (in both [`CheckTx`](../core/baseapp.md#checktx) and [`DeliverTx`](../core/baseapp.md#delivertx)), in order to discard obviously invalid messages. `ValidateBasic` should only include *stateless* checks, i.e. checks that do not require access to the state. This usually consists in checking that the message's parameters are correctly formatted and valid (i.e. that the `amount` is strictly positive for a transfer).
+- [`ValidateBasic() error`](../basics/tx-lifecycle.md#ValidateBasic).
 - `GetSignBytes() []byte`: Return the canonical byte representation of the message. Used to generate a signature.
 - `GetSigners() []AccAddress`: Return the list of signers. The Cosmos SDK will make sure that each `message` contained in a transaction is signed by all the signers listed in the list returned by this method.
 

docs/building-modules/module-interfaces.md

@@ -35,7 +35,7 @@ In general, the getter function does the following:
  - **RunE:** Defines a function that can return an error. This is the function that is called when the command is executed. This function encapsulates all of the logic to create a new transaction.
  - The function typically starts by getting the `clientCtx`, which can be done with `client.GetClientTxContext(cmd)`. The `clientCtx` contains information relevant to transaction handling, including information about the user. In this example, the `clientCtx` is used to retrieve the address of the sender by calling `clientCtx.GetFromAddress()`.
  - If applicable, the command's arguments are parsed. In this example, the arguments `[to_address]` and `[amount]` are both parsed.
- - A [message](./messages-and-queries.md) is created using the parsed arguments and information from the `clientCtx`. The constructor function of the message type is called directly. In this case, `types.NewMsgSend(fromAddr, toAddr, amount)`. Its good practice to call `msg.ValidateBasic()` after creating the message, which runs a sanity check on the provided arguments.
+ - A [message](./messages-and-queries.md) is created using the parsed arguments and information from the `clientCtx`. The constructor function of the message type is called directly. In this case, `types.NewMsgSend(fromAddr, toAddr, amount)`. Its good practice to call [`msg.ValidateBasic()`](../basics/tx-lifecycle.md#ValidateBasic) and other validation methods before broadcasting the message.
  - Depending on what the user wants, the transaction is either generated offline or signed and broadcasted to the preconfigured node using `tx.GenerateOrBroadcastTxCLI(clientCtx, flags, msg)`.
 - **Adds transaction flags:** All transaction commands must add a set of transaction [flags](#flags). The transaction flags are used to collect additional information from the user (e.g. the amount of fees the user is willing to pay). The transaction flags are added to the constructed command using `AddTxFlagsToCmd(cmd)`.
 - **Returns the command:** Finally, the transaction command is returned.

docs/building-modules/msg-services.md

@@ -29,20 +29,48 @@ When possible, the existing module's [`Keeper`](keeper.md) should implement `Msg
 
 +++ https://github.com/cosmos/cosmos-sdk/blob/v0.40.0-rc1/x/bank/keeper/msg_server.go#L27-L28
 
-`sdk.Msg` processing usually follows these 2 steps:
+`sdk.Msg` processing usually follows these 3 steps:
 
-- First, they perform *stateful* checks to make sure the `message` is valid. At this stage, the `message`'s `ValidateBasic()` method has already been called, meaning *stateless* checks on the message (like making sure parameters are correctly formatted) have already been performed. Checks performed in the `msgServer` method can be more expensive and require access to the state. For example, a `msgServer` method for a `transfer` message might check that the sending account has enough funds to actually perform the transfer. To access the state, the `msgServer` method needs to call the [`keeper`'s](./keeper.md) getter functions.
-- Then, if the checks are successful, the `msgServer` method calls the [`keeper`'s](./keeper.md) setter functions to actually perform the state transition.
+### Validation 
 
-Before returning, `msgServer` methods generally emit one or more [events](../core/events.md) via the `EventManager` held in the `ctx`:
+Before a `msgServer` method is executed, the `message`'s [`ValidateBasic()`](../basics/tx-lifecycle.md#ValidateBasic) method has already been called. Since `msg.ValidateBasic()` performs only the most basic checks, at this stage we must perform all other validation (both *stateful* and *stateless*) to make sure the `message` is valid. Checks performed in the `msgServer` method can be more expensive and the signer will be charged gas for these operations.
+For example, a `msgServer` method for a `transfer` message might check that the sending account has enough funds to actually perform the transfer. 
+
+It is recommended to implement all validation checks in a separate function, with state values passed as arguments. This will simplify testing. Expensive validation functions should charge additional gas. Example:
+
+```go
+ValidateMsgA(msg MsgA, now Time, gm GasMeter) error {
+ if now.Before(msg.Expire) {
+ return sdkerrrors.ErrInvalidRequest.Wrap("msg expired")
+ }
+ gm.ConsumeGas(1000, "signature verification")
+ return signatureVerificaton(msg.Prover, msg.Data)
+}
+```
+
+### State Transition
+
+Once the validation is successful, the `msgServer` method uses [`keeper`'s](./keeper.md) functions to access the state and perform a state transition.
+
+### Events 
+
+Before returning, `msgServer` methods generally emit one or more [events](../core/events.md) via the `EventManager` held in the `ctx`. We can use the new `EmitTypedEvent` function, which uses protobuf based event types:
+
+```
+ctx.EventManager().EmitTypedEvent(
+ &group.EventABC{Key1: Value1, Key2, Value2})
+```
+
+or the older `EmitEvent` function: 
 
 ```go
 ctx.EventManager().EmitEvent(
- sdk.NewEvent(
- eventType, // e.g. sdk.EventTypeMessage for a message, types.CustomEventType for a custom event defined in the module
- sdk.NewAttribute(attributeKey, attributeValue),
- ),
- )
+ sdk.NewEvent(
+ eventType, // e.g. sdk.EventTypeMessage for a message, types.CustomEventType for a custom event defined in the module
+ sdk.NewAttribute(key1, value1),
+ sdk.NewAttribute(key2, value2),
+ ),
+)
 ```
 
 These events are relayed back to the underlying consensus engine and can be used by service providers to implement services around the application. Click [here](../core/events.md) to learn more about events.

docs/core/baseapp.md

@@ -224,7 +224,9 @@ transaction is received by a full-node. The role of `CheckTx` is to guard the fu
 Unconfirmed transactions are relayed to peers only if they pass `CheckTx`.
 
 `CheckTx()` can perform both _stateful_ and _stateless_ checks, but developers should strive to
-make them lightweight. In the Cosmos SDK, after [decoding transactions](./encoding.md), `CheckTx()` is implemented
+make them **lightweight** because nobody is paying gas fees for the resources (CPU, data load...) used during the `CheckTx`. 
+
+In the Cosmos SDK, after [decoding transactions](./encoding.md), `CheckTx()` is implemented
 to do the following checks:
 
 1. Extract the `sdk.Msg`s from the transaction.
@@ -237,9 +239,8 @@ to do the following checks:
  as `sdk.Msg`s are not processed. Usually, the [`AnteHandler`](../basics/gas-fees.md#antehandler) will check that the `gas` provided
  with the transaction is superior to a minimum reference gas amount based on the raw transaction size,
  in order to avoid spam with transactions that provide 0 gas.
-4. Ensure that each `sdk.Msg`'s fully-qualified service method matches on of the routes inside the `msgServiceRouter`, but do **not** actually
- process `sdk.Msg`s. `sdk.Msg`s only need to be processed when the canonical state need to be updated,
- which happens during `DeliverTx`.
+
+`CheckTx` does **not** process `sdk.Msg`s - they only need to be processed when the canonical state need to be updated, which happens during `DeliverTx`.
 
 Steps 2. and 3. are performed by the [`AnteHandler`](../basics/gas-fees.md#antehandler) in the [`RunTx()`](#runtx-antehandler-and-runmsgs)
 function, which `CheckTx()` calls with the `runTxModeCheck` mode. During each step of `CheckTx()`, a
@@ -269,7 +270,7 @@ The response contains:
 #### RecheckTx
 
 After `Commit`, `CheckTx` is run again on all transactions that remain in the node's local mempool
-after filtering those included in the block. To prevent the mempool from rechecking all transactions
+excluding those included in the block. To prevent the mempool from rechecking all transactions
 every time a block is committed, the configuration option `mempool.recheck=false` can be set. As of
 Tendermint v0.32.1, an additional `Type` parameter is made available to the `CheckTx` function that
 indicates whether an incoming transaction is new (`CheckTxType_New`), or a recheck (`CheckTxType_Recheck`).

docs/core/transactions.md

@@ -25,7 +25,7 @@ Transaction objects are Cosmos SDK types that implement the `Tx` interface
 It contains the following methods:
 
 - **GetMsgs:** unwraps the transaction and returns a list of contained `sdk.Msg`s - one transaction may have one or multiple messages, which are defined by module developers.
-- **ValidateBasic:** includes lightweight, [_stateless_](../basics/tx-lifecycle.md#types-of-checks) checks used by ABCI messages [`CheckTx`](./baseapp.md#checktx) and [`DeliverTx`](./baseapp.md#delivertx) to make sure transactions are not invalid. For example, the [`auth`](https://github.com/cosmos/cosmos-sdk/tree/master/x/auth) module's `StdTx` `ValidateBasic` function checks that its transactions are signed by the correct number of signers and that the fees do not exceed what the user's maximum. Note that this function is to be distinct from the `ValidateBasic` functions for `sdk.Msg`s, which perform basic validity checks on messages only. For example, when [`runTx`](./baseapp.md#runtx) is checking a transaction created from the [`auth`](https://github.com/cosmos/cosmos-sdk/tree/master/x/auth/spec) module, it first runs `ValidateBasic` on each message, then runs the `auth` module AnteHandler which calls `ValidateBasic` for the transaction itself.
+- **ValidateBasic:** lightweight, [_stateless_](../basics/tx-lifecycle.md#types-of-checks) checks used by ABCI messages [`CheckTx`](./baseapp.md#checktx) and [`DeliverTx`](./baseapp.md#delivertx) to make sure transactions are not invalid. For example, the [`auth`](https://github.com/cosmos/cosmos-sdk/tree/master/x/auth) module's `StdTx` `ValidateBasic` function checks that its transactions are signed by the correct number of signers and that the fees do not exceed what the user's maximum. Note that this function is to be distinct from `sdk.Msg` [`ValidateBasic`](../basics/tx-lifecycle.md#ValidateBasic) methods, which perform basic validity checks on messages only. When [`runTx`](./baseapp.md#runtx) is checking a transaction created from the [`auth`](https://github.com/cosmos/cosmos-sdk/tree/master/x/auth/spec) module, it first runs `ValidateBasic` on each message, then runs the `auth` module AnteHandler which calls `ValidateBasic` for the transaction itself.
 
 As a developer, you should rarely manipulate `Tx` directly, as `Tx` is really an intermediate type used for transaction generation. Instead, developers should prefer the `TxBuilder` interface, which you can learn more about [below](#transaction-generation).