-
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
fix: remove legacy codes of wasm #640
Conversation
Signed-off-by: zemyblue <[email protected]>
Signed-off-by: zemyblue <[email protected]>
Signed-off-by: zemyblue <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #640 +/- ##
==========================================
+ Coverage 60.21% 60.24% +0.03%
==========================================
Files 817 815 -2
Lines 94990 94895 -95
==========================================
- Hits 57201 57173 -28
+ Misses 34320 34244 -76
- Partials 3469 3478 +9
|
Signed-off-by: zemyblue <[email protected]>
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.
LGTM
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 have added a question, please could you check.
) | ||
ctx.EventManager().EmitEvent(ourEvent) | ||
return nil | ||
return k.ClearContractAdmin(ctx, contractAddr, nil) | ||
} |
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.
Please could I check handleClearAdminProposal
When checking
cosmos/wasmd@e9156be#diff-a5c1aab521b573562de4814c157ce3c9b7564672c24f0acafffdb003de9c16fb (L159(129)-L178(142))
https://github.com/CosmWasm/wasmd/blob/main/x/wasm/keeper/proposal_handler.go#L188-L201
It seems a little different, my understanding might be wrong, but please could you double check.
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.
It looks like it works as same as wasmd's code, but I think it is better to unify the code with wasmd's.
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 know the difference with wasmd. But I thought this code is better suited to the return pattern of handleUpdateAdminProposal
.
BTW, EventTypeUpdateAdmin
and EventTypeClearAdmin
events are not defined in wasmd. I think these events seems to define only our code. And there is no event anywhere about ClearAdminProposal
and UpdateAdminProposal
. I think this events are necessary. What do you think, @shiki-tak and @loloicci ?
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 understand, thank you.
But I also think it would be better to unify the code with wasmd's.
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.
Thank you @da1suk8.
And the governance process i shandled by cosmos-sdk gov
module, and it seems there is a rule about proposal handler do not emit any events of type message
anymore in cosmwasm/wasmd. So we need to remove this message type event.
Signed-off-by: zemyblue <[email protected]>
* main: fix: wasm module's FIXME in the snapshotter.go file (#649) chore(deps): Bump actions/setup-go from 3.2.1 to 3.3.0 (#650) chore(deps): Bump actions/cache from 3.0.7 to 3.0.8 (#648) fix: remove legacy codes of wasm (#640) chore(deps): Bump github.com/mattn/go-isatty from 0.0.14 to 0.0.16 (#643) chore(deps): Bump actions/cache from 3.0.6 to 3.0.7 (#642) chore: change some minor things that haven't been fixed in #549 (#635) refactor: rename x/collection events (#639) feat: add additional fields into x/collection events (#638) refactor: rename x/token events (#637) feat: add creator into x/token EventIssue (#636) chore(deps): Bump github.com/coinbase/rosetta-sdk-go (#641) chore: change `Keeper.space` type to be same with cosmos-sdk refactor: remove SetBalance and SetSupply (#629) refactor: revert changes in x/slashing proto (#627) chore(deps): Bump actions/cache from 3.0.5 to 3.0.6 (#631) chore(deps): Bump github.com/prometheus/client_golang (#632) chore(deps): Bump actions/setup-go from 2.1.4 to 3.2.1 (#624) feat: add Query/TokenClassTypeName (#622) feat: add additional information into EventXXXChanged (#621) Signed-off-by: zemyblue <[email protected]> # Conflicts: # CHANGELOG.md # client/docs/statik/statik.go # simapp/app.go # x/wasm/types/events.go
Description
QueryRouter
,Router
)Event
in the wasm proposal handlers.Motivation and context
How has this been tested?
Screenshots (if appropriate):
Checklist:
CHANGELOG.md
client/docs/swagger-ui/swagger.yaml