feat: Port x/wasm module #92

Closed
aleem1314 wants to merge 3 commits from aleem/25-wasmd into main
aleem1314 commented 2023-02-28 09:02:13 +00:00 (Migrated from github.com)

Ref: #25

Description


For contributor use:

  • 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

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ v ✰ Thanks for creating a PR! ✰ v Before smashing the submit button please review the checkboxes. v If a checkbox is n/a - please still include it but + a little note why ☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --> Ref: #25 ## Description <!-- Add a description of the changes that this PR introduces and the files that are the most critical to review. --> ______ For contributor use: - [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] Re-reviewed `Files changed` in the Github PR explorer ______ For admin use: - [ ] Added appropriate labels to PR (ex. `WIP`, `R4R`, `docs`, etc) - [ ] Reviewers assigned - [ ] Squashed all commits, uses message "Merge pull request #XYZ: [title]" ([coding standards](https://github.com/tendermint/coding/blob/master/README.md#merging-a-pr))
github-code-scanning[bot] (Migrated from github.com) reviewed 2023-02-28 09:37:26 +00:00
@ -0,0 +813,4 @@
proposalTitle, err := cmd.Flags().GetString(cli.FlagTitle)
if err != nil {
return clientCtx, proposalTitle, "", nil, err
}
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:25 +00:00

Variable $X is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Variable proposalTitle is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Show more details

## Variable `$X` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference Variable `proposalTitle` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/612)
@ -0,0 +818,4 @@
proposalDescr, err := cmd.Flags().GetString(cli.FlagDescription)
if err != nil {
return client.Context{}, proposalTitle, proposalDescr, nil, err
}
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:25 +00:00

Variable $X is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Variable proposalDescr is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Show more details

## Variable `$X` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference Variable `proposalDescr` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/613)
@ -0,0 +828,4 @@
deposit, err := sdk.ParseCoinsNormalized(depositArg)
if err != nil {
return client.Context{}, proposalTitle, proposalDescr, deposit, err
}
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:25 +00:00

Variable $X is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Variable deposit is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Show more details

## Variable `$X` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference Variable `deposit` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/614)
@ -0,0 +120,4 @@
return err
}
endpoint.ClientID, err = ibctesting.ParseClientIDFromEvents(res.GetEvents())
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:24 +00:00

Should $X be modified when an error could be returned?

Should endpoint be modified when an error could be returned?

Show more details

## Should `$X` be modified when an error could be returned? Should `endpoint` be modified when an error could be returned? [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/602)
@ -0,0 +167,4 @@
return err
}
endpoint.ConnectionID, err = ibctesting.ParseConnectionIDFromEvents(res.GetEvents())
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:24 +00:00

Should $X be modified when an error could be returned?

Should endpoint be modified when an error could be returned?

Show more details

## Should `$X` be modified when an error could be returned? Should `endpoint` be modified when an error could be returned? [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/603)
@ -0,0 +194,4 @@
}
if endpoint.ConnectionID == "" {
endpoint.ConnectionID, err = ibctesting.ParseConnectionIDFromEvents(res.GetEvents())
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:24 +00:00

Should $X be modified when an error could be returned?

Should endpoint be modified when an error could be returned?

Show more details

## Should `$X` be modified when an error could be returned? Should `endpoint` be modified when an error could be returned? [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/604)
@ -0,0 +278,4 @@
return err
}
endpoint.ChannelID, err = ibctesting.ParseChannelIDFromEvents(res.GetEvents())
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:25 +00:00

Should $X be modified when an error could be returned?

Should endpoint be modified when an error could be returned?

Show more details

## Should `$X` be modified when an error could be returned? Should `endpoint` be modified when an error could be returned? [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/605)
@ -0,0 +310,4 @@
}
if endpoint.ChannelID == "" {
endpoint.ChannelID, err = ibctesting.ParseChannelIDFromEvents(res.GetEvents())
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:25 +00:00

Should $X be modified when an error could be returned?

Should endpoint be modified when an error could be returned?

Show more details

## Should `$X` be modified when an error could be returned? Should `endpoint` be modified when an error could be returned? [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/606)
@ -0,0 +157,4 @@
checksum, err = k.wasmVM.Create(wasmCode)
if err != nil {
return 0, checksum, sdkerrors.Wrap(types.ErrCreateFailed, err.Error())
}
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:37:25 +00:00

Variable $X is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Variable checksum is likely modified and later used on error. In some cases this could result in panics due to a nil dereference

Show more details

## Variable `$X` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference Variable `checksum` is likely modified and later used on error. In some cases this could result in panics due to a nil dereference [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/615)
github-code-scanning[bot] (Migrated from github.com) reviewed 2023-02-28 09:56:29 +00:00
@ -0,0 +71,4 @@
func (coord *Coordinator) UpdateTime() {
for _, chain := range coord.Chains {
coord.UpdateTimeForChain(chain)
}
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:56:28 +00:00

Iteration over map

Iteration over map may be a possible source of non-determinism

Show more details

## Iteration over map Iteration over map may be a possible source of non-determinism [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/624)
@ -0,0 +7,4 @@
"encoding/hex"
"fmt"
"math"
"reflect"
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:56:28 +00:00

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism

Show more details

## Sensitive package import Certain system packages contain functions which may be a possible source of non-determinism [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/630)
@ -0,0 +2,4 @@
import (
"encoding/json"
"reflect"
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:56:29 +00:00

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism

Show more details

## Sensitive package import Certain system packages contain functions which may be a possible source of non-determinism [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/631)
@ -0,0 +73,4 @@
if err != nil {
return nil, err
}
if rsp == nil || reflect.ValueOf(rsp).IsNil() {
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:56:28 +00:00

Impossible interface nil check

This value can never be nil, since it is a wrapped interface value.

Show more details

## Impossible interface nil check This value can never be nil, since it is a wrapped interface value. [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/634)
@ -0,0 +2,4 @@
import (
"fmt"
"reflect"
github-code-scanning[bot] (Migrated from github.com) commented 2023-02-28 09:56:29 +00:00

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism

Show more details

## Sensitive package import Certain system packages contain functions which may be a possible source of non-determinism [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/632)
i-norden reviewed 2023-03-02 21:08:58 +00:00
i-norden left a comment
Member

Thanks @aleem1314! I am wondering if we can get away with using WASM tooling as dependencies rather than internalizing a stateful module for them.

Background here is relevant here too. WASM execution environment is another piece necessary to fulfill the goal of "On-chain watcher fraud proof evaluation", it is this environment that we will need to provide access to the Postgres IPLD blockstore through the package discussed there.

Tagging @AFDudley here to discuss too.

Based on my current understanding the WASM execution environment we will use in laconicd will not need to write state into the Tendermint/Cosmos state commitment (IAVL). Service providers will usually run watcher WASM "off-chain", in watcher groups, and only when someone submits a fraud proof to Laconic does the laconicd state machine execute WASM to evaluate the fraud proof by re-running the challenged watcher state transitions to see if the result matches the result provided. But, are the state changes affected during this evaluation committed back to Tendermint? Or are they executed in a sort of speculative manner and the only thing committed is a signed attestation to the result of the evaluation?

Thanks @aleem1314! I am wondering if we can get away with using WASM tooling as dependencies rather than internalizing a stateful module for them. Background [here](https://github.com/cerc-io/laconicd/issues/24#issuecomment-1450330909) is relevant here too. WASM execution environment is another piece necessary to fulfill the goal of "On-chain watcher fraud proof evaluation", it is this environment that we will need to provide access to the Postgres IPLD blockstore through the package discussed there. Tagging @AFDudley here to discuss too. Based on my current understanding the WASM execution environment we will use in laconicd will not need to write state into the Tendermint/Cosmos state commitment (IAVL). Service providers will usually run watcher WASM "off-chain", in watcher groups, and only when someone submits a fraud proof to Laconic does the laconicd state machine execute WASM to evaluate the fraud proof by re-running the challenged watcher state transitions to see if the result matches the result provided. But, are the state changes affected during this evaluation committed back to Tendermint? Or are they executed in a sort of speculative manner and the only thing committed is a signed attestation to the result of the evaluation?
Owner

I think the result needs to be written to the chain, but we will need to use IPLD and probably make some other changes so that it's efficient to store.

I think the result needs to be written to the chain, but we will need to use IPLD and probably make some other changes so that it's efficient to store.
aleem1314 commented 2023-03-07 12:11:29 +00:00 (Migrated from github.com)

Based on my current understanding the WASM execution environment we will use in laconicd will not need to write state into the Tendermint/Cosmos state commitment (IAVL).

The CosmWasmVM Instantiate and Execute ref methods requires access to the KVStore. If we keep wasm not as a stateful module, which module store we are going to use in the wasm?

> Based on my current understanding the WASM execution environment we will use in laconicd will not need to write state into the Tendermint/Cosmos state commitment (IAVL). The CosmWasmVM `Instantiate` and `Execute` [ref](https://github.com/CosmWasm/wasmvm/blob/main/spec/Specification.md#function-definitions) methods requires access to the `KVStore`. If we keep wasm not as a stateful module, which module store we are going to use in the `wasm`?
First-time contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.
zramsay closed this pull request 2024-04-03 11:53:33 +00:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cerc-io/laconicd-deprecated#92
No description provided.