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

docs: update ADR 27 #5815

Merged
merged 5 commits into from
Mar 11, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 47 additions & 58 deletions docs/architecture/adr-027-ibc-wasm.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@

- 26/11/2020: Initial Draft
- 26/05/2023: Update after 02-client refactor and re-implementation by Strangelove
- 13/12/2023: Update after upstreaming of module to ibc-go

## Status

*Draft, needs updates*
*Accepted and applied in v0.1.0 of 08-wasm*

## Abstract

Comment on lines 4 to 14
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The term "Wasm based" should be hyphenated to "Wasm-based" for grammatical correctness.

- Wasm based light client
+ Wasm-based light client

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [15-15]

Consider adding a comma after "SDK" for better readability and changing "hardcoded" to "hard-coded" for consistency.

- In the Cosmos SDK light clients are currently hardcoded in Go.
+ In the Cosmos SDK, light clients are currently hard-coded in Go.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [16-16]

The phrase "a multi step process" should be hyphenated to "a multi-step process" for grammatical correctness.

- a multi step process
+ a multi-step process

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [32-32]

The phrase "time consuming" should be hyphenated to "time-consuming" for grammatical correctness.

- time consuming process
+ time-consuming process

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [151-151]

The term "initialise" is a valid spelling in British English, but considering the document may target an international audience, aligning with American English spelling could improve consistency.

- initialise
+ initialize

Expand All @@ -23,8 +24,8 @@ corresponding hard-fork event.
Currently in ibc-go light clients are defined as part of the codebase and are implemented as modules under
`modules/light-clients`. Adding support for new light clients or updating an existing light client in the event
of a security issue or consensus update is a multi-step process which is both time consuming and error prone.
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
In order to enable new IBC light client implementations it is necessary to modify the codebase of ibc-go,
re-build chains' binaries, pass a governance proposal and validators upgrade their nodes.
In order to enable new IBC light client implementations it is necessary to modify the codebase of ibc-go (if the light
client is part of its codebase), re-build chains' binaries, pass a governance proposal and validators upgrade their nodes.

Another problem stemming from the above process is that if a chain wants to upgrade its own consensus, it will
need to convince every chain or hub connected to it to upgrade its light client in order to stay connected. Due
Expand Down Expand Up @@ -54,13 +55,15 @@ clientKeeper.SetParams(ctx, params)

Adding a new light client contract is governance-gated. To upload a new light client users need to submit
a [governance v1 proposal](https://docs.cosmos.network/main/modules/gov#proposals) that contains the `sdk.Msg` for storing
the Wasm contract's bytecode. The required message is `MsgStoreCode` and the bytecode is provided in the field `code`:
the Wasm contract's bytecode. The required message is `MsgStoreCode` and the bytecode is provided in the field `wasm_byte_code`:

```proto
// MsgStoreCode defines the request type for the StoreCode rpc.
message MsgStoreCode {
// signer address
string signer = 1;
bytes code = 2;
// wasm byte code of light client contract. It can be raw or gzip compressed
bytes wasm_byte_code = 2;
}
```

Expand All @@ -70,36 +73,28 @@ submit this message (which is normally the address of the governance module).
```go
// StoreCode defines a rpc handler method for MsgStoreCode
func (k Keeper) StoreCode(goCtx context.Context, msg *types.MsgStoreCode) (*types.MsgStoreCodeResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

if k.authority != msg.Signer {
return nil, sdkerrors.Wrapf(govtypes.ErrInvalidSigner, "invalid authority: expected %s, got %s", k.authority, msg.Signer)
if k.GetAuthority() != msg.Signer {
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Signer)
}

codeHash, err := k.storeWasmCode(ctx, msg.Code)
ctx := sdk.UnwrapSDKContext(goCtx)
checksum, err := k.storeWasmCode(ctx, msg.WasmByteCode, ibcwasm.GetVM().StoreCode)
if err != nil {
return nil, sdkerrors.Wrap(err, "storing wasm code failed")
return nil, errorsmod.Wrap(err, "failed to store wasm bytecode")
}

ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
clienttypes.EventTypeStoreWasmCode,
sdk.NewAttribute(clienttypes.AttributeKeyWasmCodeHash, hex.EncodeToString(codeHash)),
),
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(sdk.AttributeKeyModule, clienttypes.AttributeValueCategory),
),
})
emitStoreWasmCodeEvent(ctx, checksum)

return &types.MsgStoreCodeResponse{
CodeHash: codeHash,
Checksum: checksum,
}, nil
}
```

The contract's bytecode is stored in state in an entry indexed by the code hash: `codeHash/{code hash}`. The code hash is simply
the hash of the bytecode of the contract.
The contract's bytecode is not stored in state (it is actually unnecessary and wasteful to store it, since
the Wasm VM already stores it and can be queried back, if needed). The checksum is simply the hash of the bytecode
of the contract and it is stored in state in an entry with key `checksums` that contains a list of the checksums
of bytecodes that have been stored.
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

### How light client proxy works?

Expand All @@ -108,50 +103,44 @@ in JSON format with appropriate environment information. Data returned by the sm
returned to the caller.

Consider the example of the `VerifyClientMessage` function of `ClientState` interface. Incoming arguments are
packaged inside a payload object that is then JSON serialized and passed to `callContract`, which execute `WasmVm.Execute`
packaged inside a payload object that is then JSON serialized and passed to `queryContract`, which executes `WasmVm.Query`
and returns the slice of bytes returned by the smart contract. This data is deserialized and passed as return argument.

```go
type (
verifyClientMessageInnerPayload struct {
ClientMessage clientMessage `json:"client_message"`
}
clientMessage struct {
Header *Header `json:"header,omitempty"`
Misbehaviour *Misbehaviour `json:"misbehaviour,omitempty"`
}
verifyClientMessagePayload struct {
VerifyClientMessage verifyClientMessageInnerPayload `json:"verify_client_message"`
}
)
type QueryMsg struct {
Status *StatusMsg `json:"status,omitempty"`
ExportMetadata *ExportMetadataMsg `json:"export_metadata,omitempty"`
TimestampAtHeight *TimestampAtHeightMsg `json:"timestamp_at_height,omitempty"`
VerifyClientMessage *VerifyClientMessageMsg `json:"verify_client_message,omitempty"`
CheckForMisbehaviour *CheckForMisbehaviourMsg `json:"check_for_misbehaviour,omitempty"`
}

type verifyClientMessageMsg struct {
ClientMessage *ClientMessage `json:"client_message"`
}

// VerifyClientMessage must verify a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update.
// It must handle each type of ClientMessage appropriately. Calls to CheckForMisbehaviour, UpdateState, and UpdateStateOnMisbehaviour
// will assume that the content of the ClientMessage has been verified and can be trusted. An error should be returned
// VerifyClientMessage must verify a ClientMessage.
// A ClientMessage could be a Header, Misbehaviour, or batch update.
// It must handle each type of ClientMessage appropriately.
// Calls to CheckForMisbehaviour, UpdateStaåte, and UpdateStateOnMisbehaviour
// will assume that the content of the ClientMessage has been verified
// and can be trusted. An error should be returned
// if the ClientMessage fails to verify.
func (cs ClientState) VerifyClientMessage(
ctx sdk.Context,
_ codec.BinaryCodec,
clientStore sdk.KVStore,
ctx sdk.Context,
_ codec.BinaryCodec,
clientStore storetypes.KVStore,
clientMsg exported.ClientMessage
) error {
clientMsgConcrete := clientMessage{
Header: nil,
Misbehaviour: nil,
clientMessage, ok := clientMsg.(*ClientMessage)
if !ok {
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected type: %T, got: %T", &ClientMessage{}, clientMsg)
}
switch clientMsg := clientMsg.(type) {
case *Header:
clientMsgConcrete.Header = clientMsg
case *Misbehaviour:
clientMsgConcrete.Misbehaviour = clientMsg
}
inner := verifyClientMessageInnerPayload{
ClientMessage: clientMsgConcrete,
}
payload := verifyClientMessagePayload{
VerifyClientMessage: inner,

payload := QueryMsg{
VerifyClientMessage: &VerifyClientMessageMsg{ClientMessage: clientMessage.Data},
}
_, err := call[contractResult](ctx, clientStore, &cs, payload)
_, err := wasmQuery[EmptyResult](ctx, clientStore, &cs, payload)
return err
}
```
Expand Down
Loading