fix(solecs): restrict write access to Set and MapSet to owner #244
Conversation
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
Kooshaba
approved these changes
Nov 11, 2022
|
We should probably have a way for the current owner to update the owner, right? If so, we might consider inheriting from Ownable in one of the standard set of solidity libraries (OpenZeppelin, etc.) |
|
Agree, waiting for #239 before this refactor |
|
Can we store ownership in ECS too? no reason to have state that doesn't
conform to our way of storing state. we'll need to have custom getters
client side in case someone builds a frontend for system/component
management using MUD.
I think it's good to expose view functions that implements IOwnable; but
let's avoid saving the state in the systems/components with a custom
storage slot.
…On Fri, 11 Nov 2022 at 07:32, alvarius ***@***.***> wrote:
Agree, waiting for #239 <#239>
before this refactor
—
Reply to this email directly, view it on GitHub
<#244 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFBYUK3BCSL2GYR3SJB2U3DWHZRKDANCNFSM6AAAAAAR5RUCTA>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Or if we don't do that; we can standardise the fact that only components
can write to their map set (which would make sense); and let Components
handle access control themselves with that state stored in ECS.
Mental model here is that all state that could possibly be needed by a CLI
/ a frontend should be in ECS.
…On Fri, 11 Nov 2022 at 12:53, Ludens ♝ ***@***.***> wrote:
Can we store ownership in ECS too? no reason to have state that doesn't
conform to our way of storing state. we'll need to have custom getters
client side in case someone builds a frontend for system/component
management using MUD.
I think it's good to expose view functions that implements IOwnable; but
let's avoid saving the state in the systems/components with a custom
storage slot.
On Fri, 11 Nov 2022 at 07:32, alvarius ***@***.***> wrote:
> Agree, waiting for #239 <#239>
> before this refactor
>
> —
> Reply to this email directly, view it on GitHub
> <#244 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AFBYUK3BCSL2GYR3SJB2U3DWHZRKDANCNFSM6AAAAAAR5RUCTA>
> .
> You are receiving this because your review was requested.Message ID:
> ***@***.***>
>
|
LPSCRYPT
pushed a commit
to LPSCRYPT/esp
that referenced
this pull request
Jan 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes a critical security bug in solecs (restricting write access to internal MapSet and Set contracts to the owner of those contracts)