-
Notifications
You must be signed in to change notification settings - Fork 31
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
feat: Add A New Message For Storing A Code and Instantiating A Contract With It #122
feat: Add A New Message For Storing A Code and Instantiating A Contract With It #122
Conversation
tps:337 |
tps:331 |
x/wasm/handler.go
Outdated
err := msg.ValidateBasic() | ||
if err != nil { | ||
return nil, err | ||
} |
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.
The msg validation has already been done by caller.
https://github.com/line/lbm-sdk/blob/v1/develop/baseapp/baseapp.go#L598
x/wasm/handler.go
Outdated
ContractAddress: contractAddr, | ||
} | ||
|
||
bz, err := codec.MarshalJSONIndent(types.ModuleCdc, data) |
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.
The data of msg should be length prefixed. (according to https://github.com/line/lbm-sdk/blob/v1/develop/baseapp/baseapp.go#L689-L690)
(FYI, https://github.com/line/lbm-sdk/blob/v1/develop/x/staking/handler.go#L220)
k.cdc.MarshalBinaryLengthPrefixed()
But this contract is not kept in many modules.
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.
Isn't k.cdc
private field and hidden from handler.go
?
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.
Ah! The staking module uses types.ModuleCdc
for codec. we can use it I think.
x/wasm/handler.go
Outdated
ourEvent := sdk.NewEvent( | ||
sdk.EventTypeMessage, | ||
sdk.NewAttribute(sdk.AttributeKeyModule, ModuleName), | ||
sdk.NewAttribute(types.AttributeKeySigner, msg.Sender.String()), | ||
sdk.NewAttribute(types.AttributeKeyCodeID, fmt.Sprintf("%d", codeID)), | ||
sdk.NewAttribute(types.AttributeKeyContract, contractAddr.String()), | ||
) |
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.
I think this event cannot be distinguished with https://github.com/line/lbm-sdk/blob/v1/develop/x/wasm/handler.go#L82-L88
The other modules emit multiple event .... (FYI. https://github.com/line/lbm-sdk/blob/706963dcccc471b25aedded8e6b5b958a2bdbb6e/x/gov/handler.go#L43-L57)
tps:317 |
7c8be83 includes removing the unneeded validating of messages. Missed writing in the commit message. |
storeEvent := sdk.NewEvent( | ||
sdk.EventTypeMessage, | ||
sdk.NewAttribute(sdk.AttributeKeyModule, ModuleName), | ||
sdk.NewAttribute(types.AttributeKeySigner, msg.Sender.String()), | ||
sdk.NewAttribute(types.AttributeKeyCodeID, fmt.Sprintf("%d", codeID)), | ||
) | ||
instantiateEvent := sdk.NewEvent( | ||
sdk.EventTypeMessage, | ||
sdk.NewAttribute(sdk.AttributeKeyModule, ModuleName), | ||
sdk.NewAttribute(types.AttributeKeySigner, msg.Sender.String()), | ||
sdk.NewAttribute(types.AttributeKeyCodeID, fmt.Sprintf("%d", codeID)), | ||
sdk.NewAttribute(types.AttributeKeyContract, contractAddr.String()), | ||
) |
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.
Seem that we should define another EventType
to specify msg action details.
It seems that x/wasm
is using events incorrectly as a whole.
The original concept seems that EventTypeMessage
specifies only module and sender and the details of action will be included in another EventType
Let's correct it another issue.
* Start migration server side * Return migration response and emit events * Dispatch migrate contract messages * Rebase to 0.9 and minor updates * Review feedback * Update changelog * Add msg test
Description
This PR adds a new message for storing a wasm code and instantiating a contract with it.
This PR includes
This is part of #111. After we make a stable wasm module for v2, we will make the same message for v2 and then closes it.
docs/
) or specification (x/<module>/spec/
) (There are no docs)godoc
comments.Unreleased
section inCHANGELOG.md
(It will be generated with script, right?)Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes