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.

Sign up for GitHub

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

Mark wasm queries with module_query_safe #1915

Merged
merged 4 commits into from
Jul 3, 2024
Merged

Mark wasm queries with module_query_safe #1915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ea8662
Mark wasm queries with module_query_safe
pinosu Jun 28, 2024
6d7329c
Consume gas in build address query
pinosu Jul 2, 2024
9dd5b78
Update gas calculation
pinosu Jul 2, 2024
895e633
Update DefaultGasCostBuildAddress value
pinosu Jul 2, 2024
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
13 changes: 13 additions & 0 deletions proto/cosmwasm/wasm/v1/query.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import "gogoproto/gogo.proto";
import "cosmwasm/wasm/v1/types.proto";
import "google/api/annotations.proto";
import "cosmos/base/query/v1beta1/pagination.proto";
import "cosmos/query/v1/query.proto";
import "amino/amino.proto";
import "cosmos_proto/cosmos.proto";

Expand All @@ -17,65 +18,77 @@ service Query {
// ContractInfo gets the contract meta data
rpc ContractInfo(QueryContractInfoRequest)
returns (QueryContractInfoResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/contract/{address}";
}
// ContractHistory gets the contract code history
rpc ContractHistory(QueryContractHistoryRequest)
returns (QueryContractHistoryResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get =
"/cosmwasm/wasm/v1/contract/{address}/history";
}
// ContractsByCode lists all smart contracts for a code id
rpc ContractsByCode(QueryContractsByCodeRequest)
returns (QueryContractsByCodeResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/code/{code_id}/contracts";
}
// AllContractState gets all raw store data for a single contract
rpc AllContractState(QueryAllContractStateRequest)
returns (QueryAllContractStateResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/contract/{address}/state";
}
// RawContractState gets single key from the raw store data of a contract
rpc RawContractState(QueryRawContractStateRequest)
returns (QueryRawContractStateResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get =
"/cosmwasm/wasm/v1/contract/{address}/raw/{query_data}";
}
// SmartContractState get smart query result from the contract
rpc SmartContractState(QuerySmartContractStateRequest)
returns (QuerySmartContractStateResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get =
"/cosmwasm/wasm/v1/contract/{address}/smart/{query_data}";
}
// Code gets the binary code and metadata for a singe wasm code
rpc Code(QueryCodeRequest) returns (QueryCodeResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/code/{code_id}";
}
// Codes gets the metadata for all stored wasm codes
rpc Codes(QueryCodesRequest) returns (QueryCodesResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/code";
}

// PinnedCodes gets the pinned code ids
rpc PinnedCodes(QueryPinnedCodesRequest) returns (QueryPinnedCodesResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/codes/pinned";
}

// Params gets the module params
rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get = "/cosmwasm/wasm/v1/codes/params";
}

// ContractsByCreator gets the contracts by creator
rpc ContractsByCreator(QueryContractsByCreatorRequest)
returns (QueryContractsByCreatorResponse) {
option (cosmos.query.v1.module_query_safe) = true;
option (google.api.http).get =
"/cosmwasm/wasm/v1/contracts/creator/{creator_address}";
}

// BuildAddress builds a contract address
rpc BuildAddress(QueryBuildAddressRequest)
returns (QueryBuildAddressResponse) {
option (cosmos.query.v1.module_query_safe) = true;
Comment on lines 89 to +91
Copy link
Collaborator

Choose a reason for hiding this comment

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

This query doesn't charge any gas. The option docs say:

  1. has its gas tracked, to avoid the attack vector where no gas is accounted for on potentially high-computation queries.

I guess this query is so computationally cheap that it's not an issue, but would be good to check somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good 👀 ! let's check what @webmaster128 thinks about this

Copy link
Member

Choose a reason for hiding this comment

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

This is for creating Instantiate2 addresses, right?

I think this query should be removed as the algorithm is stateless and fixed. There are multiple implementations in Rust, TypeScript and Go. I don't see any reason to perform a query to the node for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@webmaster128 should we remove this as part of next release?
I can create a separate pr for it

Copy link
Member

Choose a reason for hiding this comment

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

Let's just create a ticket as this might open a can of worms looking at the code. IMO this is not relevant enough to delay the release. We should get CosmWasm 2.1 and other fixes in wasmd out as soon as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Would't it be easy to charge a small amount of gas here for the sake of consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I think it makes sense to charge some gas for this. I will update the pr

option (google.api.http).get = "/cosmwasm/wasm/v1/contract/build_address";
}
}
Expand Down
2 changes: 1 addition & 1 deletion x/wasm/keeper/contract_keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"
"testing"

wasmvmtypes "github.com/CosmWasm/wasmvm/v2/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand All @@ -17,7 +18,6 @@ import (
"github.com/CosmWasm/wasmd/x/wasm/keeper/testdata"
"github.com/CosmWasm/wasmd/x/wasm/keeper/wasmtesting"
"github.com/CosmWasm/wasmd/x/wasm/types"
wasmvmtypes "github.com/CosmWasm/wasmvm/v2/types"
)

func TestInstantiate2(t *testing.T) {
Expand Down
7 changes: 7 additions & 0 deletions x/wasm/keeper/querier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import (
"github.com/CosmWasm/wasmd/x/wasm/types"
)

// DefaultGasCostBuildAddress is the SDK gas cost to build a contract address
const DefaultGasCostBuildAddress = 10

var _ types.QueryServer = &GrpcQuerier{}

type GrpcQuerier struct {
Expand Down Expand Up @@ -428,6 +431,10 @@ func (q GrpcQuerier) BuildAddress(c context.Context, req *types.QueryBuildAddres
if len(salt) == 0 {
return nil, status.Error(codes.InvalidArgument, "empty salt")
}

ctx := sdk.UnwrapSDKContext(c)
defer ctx.GasMeter().ConsumeGas(DefaultGasCostBuildAddress, "build address")

if req.InitArgs == nil {
return &types.QueryBuildAddressResponse{
Address: BuildContractAddressPredictable(codeHash, creator, salt, []byte{}).String(),
Expand Down
Loading