-
Notifications
You must be signed in to change notification settings - Fork 643
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
chore: use context.Context and appmodule.Environment in 08-wasm #7880
Open
technicallyty
wants to merge
19
commits into
main
Choose a base branch
from
technicallyty/environment-wasm
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
a056ce0
core modules
technicallyty 004daf5
Merge remote-tracking branch 'origin/main' into tyler/core-services
technicallyty 0953f74
fmt
technicallyty 09fac25
fix broken tests
technicallyty 9c8ab60
lint fix
technicallyty 1f838ac
actually, we can do wasm for now. this will work fine.
technicallyty 2132fe5
appmodulev2 -> appmodule
technicallyty 93b2685
environment patch
technicallyty 3dd1f33
ok most migrated now
technicallyty b4bf84e
use branch service isntead of query contexts
technicallyty 2d759ef
wip gas
technicallyty 15f51bd
migration complete
technicallyty c90669f
linter, error checks, etc etc
technicallyty 5b82400
import ordering
technicallyty 5ba53b8
linter
technicallyty 90c40f8
update queryHandler commentg
technicallyty e64bf5e
comment
technicallyty 54aa3b4
fix gas check
technicallyty 44b3ce1
Merge branch 'main' into technicallyty/environment-wasm
technicallyty File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,24 +4,24 @@ import ( | |
"bytes" | ||
"context" | ||
"encoding/hex" | ||
"fmt" | ||
|
||
wasmvm "github.com/CosmWasm/wasmvm/v2" | ||
|
||
"cosmossdk.io/collections" | ||
"cosmossdk.io/core/store" | ||
"cosmossdk.io/core/appmodule" | ||
"cosmossdk.io/core/log" | ||
errorsmod "cosmossdk.io/errors" | ||
"cosmossdk.io/log" | ||
|
||
"github.com/cosmos/cosmos-sdk/codec" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
||
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" | ||
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" | ||
"github.com/cosmos/ibc-go/v9/modules/core/exported" | ||
) | ||
|
||
// Keeper defines the 08-wasm keeper | ||
type Keeper struct { | ||
appmodule.Environment | ||
// implements gRPC QueryServer interface | ||
types.QueryServer | ||
|
||
|
@@ -30,8 +30,7 @@ type Keeper struct { | |
|
||
vm types.WasmEngine | ||
|
||
checksums collections.KeySet[[]byte] | ||
storeService store.KVStoreService | ||
checksums collections.KeySet[[]byte] | ||
|
||
queryPlugins QueryPlugins | ||
|
||
|
@@ -49,12 +48,8 @@ func (k Keeper) GetAuthority() string { | |
} | ||
|
||
// Logger returns a module-specific logger. | ||
func (Keeper) Logger(ctx sdk.Context) log.Logger { | ||
return moduleLogger(ctx) | ||
} | ||
|
||
func moduleLogger(ctx sdk.Context) log.Logger { | ||
return ctx.Logger().With("module", "x/"+exported.ModuleName+"-"+types.ModuleName) | ||
func (k Keeper) Logger(_ context.Context) log.Logger { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the context still there due to some interface that needs to be satisifed? |
||
return k.Environment.Logger | ||
} | ||
|
||
// GetVM returns the keeper's vm engine. | ||
|
@@ -77,8 +72,8 @@ func (k *Keeper) setQueryPlugins(plugins QueryPlugins) { | |
k.queryPlugins = plugins | ||
} | ||
|
||
func (k Keeper) newQueryHandler(ctx sdk.Context, callerID string) *queryHandler { | ||
return newQueryHandler(ctx, k.getQueryPlugins(), callerID) | ||
func (k Keeper) newQueryHandler(ctx context.Context, callerID string) *queryHandler { | ||
return newQueryHandler(ctx, k.Environment, k.getQueryPlugins(), callerID) | ||
} | ||
|
||
// storeWasmCode stores the contract to the VM, pins the checksum in the VM's in memory cache and stores the checksum | ||
|
@@ -87,10 +82,12 @@ func (k Keeper) newQueryHandler(ctx sdk.Context, callerID string) *queryHandler | |
// - Size bounds are checked. Contract length must not be 0 or exceed a specific size (maxWasmSize). | ||
// - The contract must not have already been stored in store. | ||
func (k Keeper) storeWasmCode(ctx context.Context, code []byte, storeFn func(code wasmvm.WasmCode, gasLimit uint64) (wasmvm.Checksum, uint64, error)) ([]byte, error) { | ||
sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917 | ||
meter := k.GasService.GasMeter(ctx) | ||
var err error | ||
if types.IsGzip(code) { | ||
sdkCtx.GasMeter().ConsumeGas(types.VMGasRegister.UncompressCosts(len(code)), "Uncompress gzip bytecode") | ||
if err := meter.Consume(types.VMGasRegister.UncompressCosts(len(code)), "Uncompress gzip bytecode"); err != nil { | ||
return nil, fmt.Errorf("failed to consume gas for Uncompress gzip bytecode: %w", err) | ||
} | ||
code, err = types.Uncompress(code, types.MaxWasmSize) | ||
if err != nil { | ||
return nil, errorsmod.Wrap(err, "failed to store contract") | ||
|
@@ -113,9 +110,9 @@ func (k Keeper) storeWasmCode(ctx context.Context, code []byte, storeFn func(cod | |
} | ||
|
||
// create the code in the vm | ||
gasLeft := types.VMGasRegister.RuntimeGasForContract(sdkCtx) | ||
gasLeft := types.VMGasRegister.RuntimeGasForContract(meter) | ||
vmChecksum, gasUsed, err := storeFn(code, gasLeft) | ||
types.VMGasRegister.ConsumeRuntimeGas(sdkCtx, gasUsed) | ||
types.VMGasRegister.ConsumeRuntimeGas(meter, gasUsed) | ||
if err != nil { | ||
return nil, errorsmod.Wrap(err, "failed to store contract") | ||
} | ||
|
@@ -141,7 +138,7 @@ func (k Keeper) storeWasmCode(ctx context.Context, code []byte, storeFn func(cod | |
|
||
// migrateContractCode migrates the contract for a given light client to one denoted by the given new checksum. The checksum we | ||
// are migrating to must first be stored using storeWasmCode and must not match the checksum currently stored for this light client. | ||
func (k Keeper) migrateContractCode(ctx sdk.Context, clientID string, newChecksum, migrateMsg []byte) error { | ||
func (k Keeper) migrateContractCode(ctx context.Context, clientID string, newChecksum, migrateMsg []byte) error { | ||
clientStore := k.clientKeeper.ClientStore(ctx, clientID) | ||
wasmClientState, found := types.GetClientState(clientStore, k.cdc) | ||
if !found { | ||
|
@@ -180,13 +177,15 @@ func (k Keeper) migrateContractCode(ctx sdk.Context, clientID string, newChecksu | |
|
||
k.clientKeeper.SetClientState(ctx, clientID, wasmClientState) | ||
|
||
emitMigrateContractEvent(ctx, clientID, oldChecksum, newChecksum) | ||
if err = emitMigrateContractEvent(k.EventService.EventManager(ctx), clientID, oldChecksum, newChecksum); err != nil { | ||
return fmt.Errorf("failed to emit migrate contract events: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// GetWasmClientState returns the 08-wasm client state for the given client identifier. | ||
func (k Keeper) GetWasmClientState(ctx sdk.Context, clientID string) (*types.ClientState, error) { | ||
func (k Keeper) GetWasmClientState(ctx context.Context, clientID string) (*types.ClientState, error) { | ||
clientState, found := k.clientKeeper.GetClientState(ctx, clientID) | ||
if !found { | ||
return nil, errorsmod.Wrapf(clienttypes.ErrClientTypeNotFound, "clientID %s", clientID) | ||
|
@@ -233,7 +232,7 @@ func (k Keeper) HasChecksum(ctx context.Context, checksum types.Checksum) bool { | |
} | ||
|
||
// InitializePinnedCodes updates wasmvm to pin to cache all contracts marked as pinned | ||
func (k Keeper) InitializePinnedCodes(ctx sdk.Context) error { | ||
func (k Keeper) InitializePinnedCodes(ctx context.Context) error { | ||
checksums, err := k.GetAllChecksums(ctx) | ||
if err != nil { | ||
return err | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this, we have done another approach in transfer which I think is a little better: make the function a receiver function on the keeper and just pass in the context:
func (k Keeper) EmitTransferEvent(ctx context.Context