Improve documentation on x/circuit default Ante Handler

[julienrbrt]

x/circuit module is a circuit breaker module meant to prohibit specific txs from being executed.
Read more in https://docs.cosmos.network/main/modules/circuit.

It works thanks to two components:

• The CircuitBreakerDecorator ante handler:
  https://github.com/cosmos/cosmos-sdk/blob/x/circuit/v0.1.0/x/circuit/ante/circuit.go#L27-L41
• A message router check:
  https://github.com/cosmos/cosmos-sdk/blob/v0.50.1/baseapp/msg_service_router.go#L104-L115

@Reecepbcups rightfully pointed that the default circuit ante handler does not check for inner messages.
That is correct, this means any messages containing inner messages (x/authz, x/gov, or any custom modules having that) will pass the ante handler but fail at CheckTx.
However, it is important to note that such messages will never get executed and will properly fail at the message router check.

The reason the CircuitBreakerDecorator does not do those checks is because, currently, it would introduce dependencies (x/authz, x/gov) in the x/circuit module and the circuit cannot (currently) know if a transaction contains inner msgs. Additionally, standalone SDK modules aim to require strictly necessary dependencies.
This means it is expected that the circuit ante handler does not perform those checks. Chains can re-define their circuit ante handler that handles all messages containing inner messages if they really wish too (because a chain can exhaustively know what are their applicable messages).

This tradeoff in the default circuit ante handler should be made clearer in the circuit docs (https://docs.cosmos.network/main/modules/circuit).

[alexanderbez]

Great summary, thanks @julienrbrt.

[Reecepbcups]

note: teams are continuing to copy paste this circuit ante handler logic and thinking their cases are covered because it's how the SDK does it. They still do not have the provided context (at a glace) that the circuit is unique in fixing this problem in the baseapp. Thus leading to security holes in their software.

2 options:

• The ante having tx.GetAllMsgs() where it auto unwraps authz into the standard would be very nice (not ideal bc authz dep)
• linking to the part of the docs / a warning NOT to copy it and provide information on authz edge cases within the x/circuit/ante/circuit.go

[julienrbrt]

Feel free to send a PR that links the docs 👍🏾
However, how is it leaving security wholes in their software? As mentioned above, the message won't pass the message router. Additionally, we've extended the docs in circuit and the default ante handler documentation: https://docs.cosmos.network/main/learn/beginner/tx-lifecycle#antehandler

[Reecepbcups]

While x/circuit is secure because of baseapp, teams are copy pasting the circuit ante handler and using it as reference for their own logic they are building (not circuit related). These networks have authz enabled.

Therefor teams are not understanding the full context of x/circuits unique way to handle this, vs what teams actually have to do if not using circuits. i.e. standard message filtering and other logically flows.

For example: https://github.com/CosmosContracts/juno/blob/v21.0.0/app/decorators/change_rate_decorator.go#L48-L64