-
Notifications
You must be signed in to change notification settings - Fork 411
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
Add missing events for admin-related methods #1177
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -639,6 +639,20 @@ func (k Keeper) setContractAdmin(ctx sdk.Context, contractAddress, caller, newAd | |
} | ||
contractInfo.Admin = newAdmin.String() | ||
k.storeContractInfo(ctx, contractAddress, contractInfo) | ||
|
||
if newAdmin != nil { | ||
ctx.EventManager().EmitEvent(sdk.NewEvent( | ||
types.EventTypeUpdateContractAdmin, | ||
sdk.NewAttribute(types.AttributeKeyContractAddr, contractAddress.String()), | ||
sdk.NewAttribute(types.AttributeKeyAdmin, newAdmin.String()), | ||
)) | ||
} else { | ||
ctx.EventManager().EmitEvent(sdk.NewEvent( | ||
types.EventTypeClearContractAdmin, | ||
sdk.NewAttribute(types.AttributeKeyContractAddr, contractAddress.String()), | ||
)) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
|
@@ -976,6 +990,14 @@ func (k Keeper) setAccessConfig(ctx sdk.Context, codeID uint64, caller sdk.AccAd | |
|
||
info.InstantiateConfig = newConfig | ||
k.storeCodeInfo(ctx, codeID, *info) | ||
|
||
ctx.EventManager().EmitEvent(sdk.NewEvent( | ||
types.EventTypeSetAccessConfig, | ||
sdk.NewAttribute(types.AttributeKeyCodeID, strconv.FormatUint(codeID, 10)), | ||
sdk.NewAttribute(types.AttributeKeyCodePermission, newConfig.Permission.String()), | ||
sdk.NewAttribute(types.AttributeKeyAuthorizedAddresses, strings.Join(newConfig.Addresses, ", ")), | ||
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. The nit: the space in the join pattern makes it easier to read for humans but adds a bit more complexity for the clients. Either they find the pattern or have to trim each address. 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.
I think it's fine to always add |
||
)) | ||
|
||
return 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.
I was thinking of 2 different events first but then merged them into 1 to make it easier for clients to subscribe to updates. When the admin is cleared, the new admin value would be empty.
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 like your option as well. I just thought about this and I decided to distinguish those events from the view of a message as a unit of action. (if the admin is cleared, nobody can update the admin after that and only gov can do this)