[DO NOT MERGE, PREVIEW ONLY] Merge `feature/cosmwasm` into `main` #2074

teddyding · 2024-08-12T12:21:02Z

Changelist

[Describe or list the changes made in this PR]

Test Plan

[Describe how this PR was tested (if applicable)]

Author/Reviewer Checklist

If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
Manually add any of the following labels: refactor, chore, bug.

…1499) * Add initial test contract with dydx messages. * Add custom querier support for OracleQuery (#974) * cleanup + remove unnecessary files * cherrypick 8e3f5b2, modified versions * Add Rust library for Dydx Query (#979) * update cargo.toml and Cargo.lock --------- Co-authored-by: Vincent Chau <[email protected]>

…#1809)

coderabbitai · 2024-08-12T12:21:07Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

chipshort

Not a full review, just some comments I had while looking at the code

chipshort · 2024-09-26T03:16:33Z

protocol/app/app.go

+	// AllCapabilities returns all capabilities available with the current wasmvm
+	// See https://github.com/CosmWasm/cosmwasm/blob/main/docs/CAPABILITIES-BUILT-IN.md
+	supportedFeatures := "iterator,staking,stargate,cosmwasm_1_1,cosmwasm_1_2,cosmwasm_1_4,dydx"
+


I'd recommend using something like strings.Join(append(AllCapabilities(),"dydx"), ","). That way you don't need to manually update the capabilities when you upgrade x/wasm.

chipshort · 2024-09-26T03:28:51Z

protocol/app/app.go

@@ -1476,6 +1557,10 @@ func New(

 	// initialize BaseApp
 	app.SetInitChainer(app.InitChainer)
+	// wasmd also comes with 2 custom ante handlers:
+	//  - CountTXDecorator adds the TX position in the block into the context and passes it to the contracts


You need this if you want contracts to get access to env.transaction. Otherwise they get None there.

chipshort · 2024-09-26T03:30:29Z

protocol/app/app.go

@@ -1476,6 +1557,10 @@ func New(

 	// initialize BaseApp
 	app.SetInitChainer(app.InitChainer)
+	// wasmd also comes with 2 custom ante handlers:
+	//  - CountTXDecorator adds the TX position in the block into the context and passes it to the contracts
+	//  - LimitSimulationGasDecorator prevents an "infinite gas" query


It's probably less important for permissioned chains, but still good to have in case someone finds a way to cause an infinite loop in a contract call.

pinosu

Added some comments

pinosu · 2024-10-02T15:47:05Z

protocol/Dockerfile

@@ -12,6 +14,7 @@ ARG GOLANG_1_22_ALPINE_DIGEST="cdc86d9f363e8786845bea2040312b4efa321b828acdeb26f
 FROM golang@sha256:${GOLANG_1_22_ALPINE_DIGEST} as builder
 ARG VERSION
 ARG COMMIT
+ARG WASMVM_VERSION=v1.5.2


It would be great to use wasmvm v1.5.5 because it includes some security fixes.

pinosu · 2024-10-02T15:52:25Z

protocol/app/ante/wasm_execute.go

+	if !isSingleWasmEx {
+		return next(ctx, tx, simulate)
+	}


Do you actually mean to skip this ante handler if there are multiple wasm exec in the same tx?
To my understanding you probably want to discard the tx, in that case you should just return an error

pinosu · 2024-10-02T16:01:35Z

protocol/app/ante.go

@@ -396,6 +404,9 @@ func (h *lockingAnteHandler) otherMsgAnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 		defer h.globalLock.Unlock()
 	}

+	if ctx, err = h.executeCosmWasm.AnteHandle(ctx, tx, simulate, noOpAnteHandle); err != nil {


you should also add wasmd ante handlers :

CountTXDecorator is useful because it provides position data to the contract via Env . The motivation is to add a unique ID to the contract environment to distinguish between multiple executions within the same block but in different tx versus a reentrant call within the same msg or tx.

LimitSimulationGasDecorator limits the maximum gas available in simulations, preventing the "infinite gas" issues.
So to avoid well known issues and keep the full compatibility both ante handlers should be provided.

pinosu · 2024-10-02T16:02:52Z

protocol/app/module_accounts.go

@@ -35,6 +36,9 @@ var (
 		govtypes.ModuleName:            {authtypes.Burner},
 		ibctransfertypes.ModuleName:    {authtypes.Minter, authtypes.Burner},
 		icatypes.ModuleName:            nil,
+		// https://github.com/CosmWasm/wasmvm/blob/1c3fdc2a4402e527617ec72fe53f114b24899a01/types/msg.go#L116-L121
+		// required to burn given coins from the contract's account.
+		wasmtypes.ModuleName: {authtypes.Burner},


uint

I reviewed the Rust bits and they look fine save for nitpicks.

uint · 2024-10-03T13:42:32Z

protocol/dydx-cosmwasm/src/query.rs

+// implement custom query
+impl CustomQuery for DydxQueryWrapper {}
+
+/// SeiQuery is defines available query datas


Suggested change

/// SeiQuery is defines available query datas

/// SeiQuery defines available query datas

uint · 2024-10-03T13:44:49Z

protocol/dydx-cosmwasm/src/serializable_int.rs

+    pub fn new(i: BigInt) -> Self {
+        Self { i }
+    }
+    pub fn to_big_int(&self) -> &BigInt {


Suggested change

pub fn to_big_int(&self) -> &BigInt {

pub fn as_big_int(&self) -> &BigInt {

This is more idiomatic for reference conversions. See example

uint · 2024-10-03T13:50:45Z

protocol/contracts/dydx-messages-example/src/contract.rs

+use dydx_cosmwasm::{DydxQuerier, DydxQueryWrapper, SubaccountId};
+use dydx_cosmwasm::DydxQuery;
+
+// version info for migration info


Suggested change

// version info for migration info

// version info for migration

uint · 2024-10-03T13:57:41Z

protocol/contracts/dydx-messages-example/src/contract.rs

+}
+
+#[cfg_attr(not(feature = "library"), entry_point)]
+pub fn query(deps: Deps<DydxQueryWrapper>, _env: Env, msg: DydxQuery) -> StdResult<Binary> {


Is this contract meant as an example for contract devs to use as a reference? I worry accepting the DydxQuery type from the bindings lib would be confusing to them. It's more typical for a contract to define its own QueryMsg.

teddyding and others added 14 commits August 9, 2024 17:24

Merged initial commits from 2024-hackathon-cosmwasm (#1497)

5b7bf65

Add custom messages to cosmwasm (#1612)

33a4351

Add custom queries to cosmwasm (#1617)

f316377

Cleanup and consolidate msg enums (#1680)

e01296a

Add documentation on getting started with dydx-cosmwasm (#1677)

3a735a8

[OTE-415] Add test for getting perpetual clob details (#1739)

896f1bb

add local testing docs (#1784)

2f0b622

space

c1e0188

[OTE-407][OTE-409][OTE-426]Update wasm bindings to use custom encoder (…

03a979b

…#1809)

Fix CI for cosmwasm integrated V4-chain (#1708)

dd62ee4

add defaults to u32 fields

3c464c8

add helper function to get big int value (#1973)

56d90f9

update versions (#1993)

9276b96

github-actions bot added indexer protocol proto labels Aug 12, 2024

teddyding changed the title ~~[DO NOT MERGE] Merge feature/cosmwasm into main~~ [DO NOT MERGE, PREVIEW ONLY] Merge feature/cosmwasm into main Aug 13, 2024

Fix sample-pregenesis and proto-format

e3331b3

teddyding force-pushed the feature/cosmwasm branch from b4828e1 to e3331b3 Compare August 20, 2024 15:40

mergify bot mentioned this pull request Aug 20, 2024

Fix upgrade container test (v6 -> vCosmWasm) #2110

Merged

6 tasks

teddyding and others added 5 commits August 20, 2024 18:15

Fix upgrade container test (v6 -> vCosmWasm) (#2110)

8d7fbcf

[OTE-699] Wasm execution rate-limit implementation (#2075)

35dcadf

[OTE-561] Blocklist cosmwasm messages not needed for MVP (#2005)

f17143a

Use Cosmos SDK gas-free feature branch (#2165)

303818c

Address 3 TODO items (#2200)

4cc07fd

chipshort reviewed Oct 1, 2024

View reviewed changes

pinosu reviewed Oct 2, 2024

View reviewed changes

uint reviewed Oct 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE, PREVIEW ONLY] Merge `feature/cosmwasm` into `main` #2074

[DO NOT MERGE, PREVIEW ONLY] Merge `feature/cosmwasm` into `main` #2074

teddyding commented Aug 12, 2024

coderabbitai bot commented Aug 12, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

chipshort left a comment

chipshort Sep 26, 2024

chipshort Sep 26, 2024

chipshort Sep 26, 2024

pinosu left a comment

pinosu Oct 2, 2024

pinosu Oct 2, 2024

pinosu Oct 2, 2024

pinosu Oct 2, 2024

uint left a comment

uint Oct 3, 2024

uint Oct 3, 2024

uint Oct 3, 2024

uint Oct 3, 2024

	/// SeiQuery is defines available query datas
	/// SeiQuery defines available query datas

	pub fn to_big_int(&self) -> &BigInt {
	pub fn as_big_int(&self) -> &BigInt {

	// version info for migration info
	// version info for migration

[DO NOT MERGE, PREVIEW ONLY] Merge feature/cosmwasm into main #2074

Are you sure you want to change the base?

[DO NOT MERGE, PREVIEW ONLY] Merge feature/cosmwasm into main #2074

Conversation

teddyding commented Aug 12, 2024

Changelist

Test Plan

Author/Reviewer Checklist

coderabbitai bot commented Aug 12, 2024 • edited Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

chipshort left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinosu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uint left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[DO NOT MERGE, PREVIEW ONLY] Merge `feature/cosmwasm` into `main` #2074

[DO NOT MERGE, PREVIEW ONLY] Merge `feature/cosmwasm` into `main` #2074

coderabbitai bot commented Aug 12, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)