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

Jump to bottom

fix: remove legacy codes of wasm #640

Merged
merged 6 commits into from
Aug 22, 2022
Merged

fix: remove legacy codes of wasm #640

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
541a3f2
remove unused interface and struct in wasm (QueryRouter, Router)
zemyblue Aug 10, 2022
f08470e
remove duplicated Event in the wasm proposal handler
zemyblue Aug 10, 2022
097412a
chore: add changelog
zemyblue Aug 10, 2022
cd5b920
fix: lint error
zemyblue Aug 10, 2022
23b9059
Merge branch 'main' into remove_legacy_codes_of_wasm
zemyblue Aug 16, 2022
3dfe8f4
chore: remove the message types event of governance process.
zemyblue Aug 18, 2022
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
58 changes: 2 additions & 56 deletions x/wasm/keeper/proposal_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@ package keeper

import (
"encoding/hex"
"strconv"
"strings"

sdk "github.com/line/lbm-sdk/types"
sdkerrors "github.com/line/lbm-sdk/types/errors"
govtypes "github.com/line/lbm-sdk/x/gov/types"
Expand Down Expand Up @@ -182,17 +179,7 @@ func handleUpdateAdminProposal(ctx sdk.Context, k types.ContractOpsKeeper, p typ
return sdkerrors.Wrap(err, "run as address")
}

if err := k.UpdateContractAdmin(ctx, contractAddr, nil, newAdminAddr); err != nil {
return err
}

ourEvent := sdk.NewEvent(
types.EventTypeUpdateAdmin,
sdk.NewAttribute(types.AttributeKeyContractAddr, p.Contract),
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
)
ctx.EventManager().EmitEvent(ourEvent)
return nil
return k.UpdateContractAdmin(ctx, contractAddr, nil, newAdminAddr)
}

func handleClearAdminProposal(ctx sdk.Context, k types.ContractOpsKeeper, p types.ClearAdminProposal) error {
Expand All @@ -204,17 +191,7 @@ func handleClearAdminProposal(ctx sdk.Context, k types.ContractOpsKeeper, p type
if err != nil {
return sdkerrors.Wrap(err, "contract")
}
if err := k.ClearContractAdmin(ctx, contractAddr, nil); err != nil {
return err
}

ourEvent := sdk.NewEvent(
types.EventTypeClearAdmin,
sdk.NewAttribute(types.AttributeKeyContractAddr, p.Contract),
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
)
ctx.EventManager().EmitEvent(ourEvent)
return nil
return k.ClearContractAdmin(ctx, contractAddr, nil)
}
Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Member Author

@zemyblue zemyblue Aug 17, 2022 •
edited
Loading

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 ?

Copy link
Member

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.

Copy link
Member Author

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.

CosmWasm/wasmd@e9156be


func handlePinCodesProposal(ctx sdk.Context, k types.ContractOpsKeeper, p types.PinCodesProposal) error {
Expand All @@ -226,22 +203,6 @@ func handlePinCodesProposal(ctx sdk.Context, k types.ContractOpsKeeper, p types.
return sdkerrors.Wrapf(err, "code id: %d", v)
}
}
s := make([]string, len(p.CodeIDs))
for _, v := range p.CodeIDs {
ourEvent := sdk.NewEvent(
types.EventTypePinCode,
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
sdk.NewAttribute(types.AttributeKeyCodeID, strconv.FormatUint(v, 10)),
)
ctx.EventManager().EmitEvent(ourEvent)
}

ourEvent := sdk.NewEvent(
types.EventTypePinCode,
sdk.NewAttribute(types.AttributeKeyCodeIDs, strings.Join(s, ",")),
)
ctx.EventManager().EmitEvent(ourEvent)

return nil
}

Expand All @@ -254,21 +215,6 @@ func handleUnpinCodesProposal(ctx sdk.Context, k types.ContractOpsKeeper, p type
return sdkerrors.Wrapf(err, "code id: %d", v)
}
}
s := make([]string, len(p.CodeIDs))
for _, v := range p.CodeIDs {
ourEvent := sdk.NewEvent(
types.EventTypeUnpinCode,
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
sdk.NewAttribute(types.AttributeKeyCodeID, strconv.FormatUint(v, 10)),
)
ctx.EventManager().EmitEvent(ourEvent)
}

ourEvent := sdk.NewEvent(
types.EventTypeUnpinCode,
sdk.NewAttribute(types.AttributeKeyCodeIDs, strings.Join(s, ",")),
)
ctx.EventManager().EmitEvent(ourEvent)

return nil
}
Expand Down