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.

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

NEP-480: Account Namespaces #480

Closed
wants to merge 11 commits into from
Closed

NEP-480: Account Namespaces #480

wants to merge 11 commits into from

Conversation

encody
Copy link

@encody encody commented May 12, 2023

@render
Copy link

render bot commented May 12, 2023

@encody encody changed the title Account Namespaces NEP-480: Account Namespaces May 12, 2023
@encody encody marked this pull request as ready for review May 12, 2023 12:23
@encody encody requested a review from a team as a code owner May 12, 2023 12:23
neps/nep-0480.md Show resolved Hide resolved
neps/nep-0480.md Outdated Show resolved Hide resolved
neps/nep-0480.md Outdated Show resolved Hide resolved
Copy link
Contributor

@akhi3030 akhi3030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the NEP also specify how the token balance is shared or not between namespaces?

@ori-near ori-near added WG-protocol Protocol Standards Work Group should be accountable S-draft/needs-implementation A NEP in the DRAFT stage that needs implementation. A-NEP A NEAR Enhancement Proposal (NEP). labels May 15, 2023
@ori-near
Copy link
Contributor

Hi @encody – thank you for starting this NEP. As the moderator, I labeled this PR as "Needs Implementation" because we saw that the reference implementation is still in progress. Please ping the @near/nep-moderators once you are ready for us to review it.

@encody
Copy link
Author

encody commented May 30, 2023

@near/nep-moderators This should be ready for another look!

Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @encody for submitting this NEP.

As a moderator, I reviewed this NEP and it meets the proposed template guidelines. I am moving this NEP to the REVIEW stage and would like to ask the @near/wg-protocol working group members to assign 2 Technical Reviewers to complete a technical review (see expectations below).

Just for clarity, Technical Reviewers play a crucial role in scaling NEAR ecosystem as they provide their in-depth expertise in the niche topic while work group members can stay on guard of the NEAR ecosystem. The discussions may get too deep and it would be inefficient for each WG member to dive into every single comment, so NEAR Developer Governance designed this process that includes subject matter experts helping us to scale by writing a summary with the raised concerns and how they were addressed.

Technical Review Guidelines
  • First, review the proposal within one week. If you have any suggestions that could be fixed, leave them as comments to the author. It may take a couple of iterations to resolve any open comments.

  • Second, once all the suggestions are addressed, produce a Technical Summary, which helps the working group members make a weighted decision faster. Without the summary, the working group will have to read the whole discussion and potentially miss some details.

Technical Summary guidelines:

  • A recommendation for the working group if the NEP is ready for voting (it could be approving or rejecting recommendation). Please note that this is the reviewer's personal recommendation.

  • A summary of benefits that surfaced in previous discussions. This should include a concise list of all the benefits that others raised, not just the ones that the reviewer personally agrees with.

  • A summary of concerns or blockers, along with their current status and resolution. Again, this should reflect the collective view of all commenters, not just the reviewer's perspective.

Here is a nice example and a template for your convenience:


### Recommendation

Add recommendation

### Benefits

* Benefit

* Benefit

### Concerns

| # | Concern | Resolution | Status |

| - | - | - | - |

| 1 | Concern | Resolution | Status |

| 2 | Concern | Resolution | Status |

Please tag the @near/nep-moderators once you are done, so we can move this NEP to the voting stage. Thanks again.

Comment on lines +106 to +108
## Drawbacks (Optional)

## Unresolved Issues (Optional)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## Drawbacks (Optional)
## Unresolved Issues (Optional)
## Drawbacks
## Unresolved Issues

@frol frol added S-review/needs-wg-to-assign-sme A NEP that needs working group to assign two SMEs. and removed S-draft/needs-implementation A NEP in the DRAFT stage that needs implementation. labels May 30, 2023
@encody encody mentioned this pull request Jun 2, 2023
2 tasks
@bowenwang1996
Copy link
Collaborator

As a working group member, I'd like to nominate @akhi3030 and @nagisa as SMEs to review this NEP.

Copy link
Contributor

@nagisa nagisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an initial pass and largely just my brain dump.

neps/nep-0480.md Show resolved Hide resolved

If code has already been deployed to the specified namespace, it is replaced, leaving code deployed to other namespaces untouched.

Once a namespace is created, this NEP does not provide a way to delete it. However, namespaces should not be considered permanent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we’d need a concept of namespace creation? Would this creation step introduce semantics meaningfully different from the empty namespace existing for accounts that haven't deployed a contract yet?

If not, I think it would be quite a bit more straightforward to work with this feature if all namespaces were presumed to “exist” with a code_hash equal to 11111111111111111111111111111111 or whatever we use for invalid contract.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Namespace creation is approximately equal to contract deployment in this NEP. If all namespaces are presumed to exist, how does an RPC account view call show namespaces?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm not sure! I imagine it would list the non-111...111 hashes, rather than the list of “namespaces”?

But counter-question to your question: what's the expected behaviour of calling a non-existent namespace? If my memory serves me right, in the namespace-less world of today, the behaviour is meaningfully different between calling a function in a non-existent account and calling a function in an existing account without a contract deployed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that case would be similar to calling a non-existent method on an account that does have a contract deployed. Possibly a case for a new, "namespace does not exist" error?


### `FunctionCallPermission`

A `namespaces` field is added to `FunctionCallPermission`. This field contains a list of namespaces that the access key is allowed to call. If the `namespaces` field is not specified, the access key is allowed to call all namespaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason I’m not seeing why this deviates from the usual “no namespace ⇒ empty namespace” defaulting elsewhere in this NEP? I feel like it might be a notable implicit foot gun in the use of protocol. Consider for example:

  1. Following a tutorial I deploy a contract to the empty (default) namespace;
  2. Since the tutorial is namespace oblivious, they suggest adding a key without namespace;
  3. Later on I deploy a different contract that requires strict access permissions to a namespace.

Forgetting I’ve done 2, I have accidentally allowed the important contract to be called arbitrarily by the old key(s).

The alternative is to equate “empty namespaces ⇒ default empty namespace” here as well, in which case the failure mode is that I attempt to call the new contract with an old key, notice it doesn't work and expand the permissions for the key.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, a function call access key that does not specify a list of methods is allowed to call any method. Therefore, a function call access key that does not specify a list of namespaces is allowed to call any namespace.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I feel like the stakes are likely somewhat different here in that deploying a new contract to an account in absence of namespaces overwrites the old one, and thus the user is conscious about both the previous and the new contract (and thus possibly is thinking of the access controls in context of both.) In the world of namespaces they no longer need to care about any previous state anymore.

But fair, I see how the proposed behaviour is consistent.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need a new syntax for function call access keys, wherein they can be permitted to call "any method on xyz namespace"?


- `predecessor_namespace() -> Namespace`: Returns the namespace of the predecessor account, which may be the empty namespace. In the case that the predecessor is not a smart contract, this is the empty namespace.
- `current_namespace() -> Namespace`: Returns the namespace of the current account, which may be the empty namespace.
- `namespace_storage_usage(&Namespace) -> u64`: Returns the storage usage of the given namespace. This includes storage consumed by the contract and state, but not account data (e.g. balance, access keys). That is to say,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the value of obtaining the storage usage of a different namespace than the one currently executing? If there isn’t one, might it make sense to require that the Namespace arugment is current_namepsace() otherwise a panic is raised for the time being?

Might be worthwhile to double check if an ability to do so does not preclude whatever other improvements we might want to make.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the suggested use-cases for namespaces is to support "hot-swapping" wherein a manager namespace deploys code to other namespaces and manages their state and execution at a high level, so some cross-namespace introspection functions like this would be useful.

neps/nep-0480.md Show resolved Hide resolved
@akhi3030
Copy link
Contributor

As a working group member, I'd like to nominate @akhi3030 and @nagisa as SMEs to review this NEP.

@near/nep-moderators: looks like SMEs are assigned so maybe we need to update the labels on the NEP.


- It is impossible for a non-developer user to compose multiple contract standards together into a single contract without actually performing some development work. Account namespaces would enable a user workflow wherein a user only needs to choose which contracts to deploy, and not worry about how to compose them.

- If a developer wants their smart contract to be upgradable, they have to manually write the upgrade functionality into their smart contract, and smart contract upgrades are complex, multi-step operations with many security considerations and opportunities to "brick" the account. Namespaces would allow a developer to deploy a semi-standardized upgrade module to a namespace, and have that manage the upgrade process for the other (default, etc.) namespaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Namespaces would allow a developer to deploy a semi-standardized upgrade module to a namespace, and have that manage the upgrade process for the other (default, etc.) namespaces.

This does not explain how namespaces enable this feature. More specifically, what is the limitation of the current protocol that is eliminated with this proposal

Copy link
Author

@encody encody Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workflow example:

  1. Alice creates alice.near.
  2. Alice deploys main contract bytecode v1 to the default namespace.
  3. Alice deploys bytecode upgrade-manager to the upgrade-manager namespace. The upgrade-manager bytecode provides an upgrade method that performs a contract upgrade sequence (explained in later steps).
  4. Alice deletes all full-access keys from alice.near.
  5. Alice creates a new version of the main contract bytecode. She calls this version v2.
  6. Alice calls alice.near:upgrade-manager->upgrade(v2), passing in the v2 bytecode.
  7. The upgrade-manager code performs a contract upgrade sequence, deploying the v2 bytecode to the default namespace of alice.near.
  8. Turns out, the v2 bytecode is invalid/unusable.
  9. Alice creates another new version of the main contract bytecode, v3.
  10. Alice calls the still-functional upgrade-manager to perform the upgrade sequence with v3.
  11. The account has a valid contract deployed to the default namespace, and continues to work as intended.

If namespaces had not existed, the v2 deployment would have bricked the entire alice.near account.


- Certain contracts are frequently deployed on different accounts. "Codeless contracts" allow an account to deploy just the hash of a smart contract instead of the full binary. This, in conjunction with namespaces, would allow a user to pick and choose a set of account features to deploy with negligible gas and storage costs.

- Permissions: It could be possible to set permissions on a particular namespace. For example, restricting the namespace `dao_multisig` to interactions with `dao.near` only.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the only capabilities that you propose to enable? What about arbitrary host functions, etc.? Having read through the proposal in more detail, I think it is important to have fine grained capabilities for namespaces. What you are proposing is to allow smart contract developers who are not proficient to deploy multiple wasm modules on their account. These wasm modules could be buggy or malicious. If this proposal is expected, I can imagine some users just having a single account with all the different contracts they want to try out / experiment with and without fine grained control over their capabilities, they could end up losing a lot of funds.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how all of the questions in this comment fit together.

Currently, I don't think that the typical non-tech-savvy user deploys any code to their accounts, but please correct me if I am wrong.

Specifically when speaking of non-tech-savvy users, the workflow we conceptualized is something like:

  1. Alice wants to allow Bob to access her funds if she is inactive for 180 days.
  2. Alice logs into her wallet.
  3. Alice clicks on "Account Extensions."
  4. Alice scrolls through the list of available extensions and selects "Dead Man Switch."
  5. Alice inputs parameters:
    • Beneficiary ID: bob.near
    • Timeframe: 180 days
  6. Alice clicks "Save."
  7. Alice approves the contract deployment + initialization function call action:
    • Namespace: dead-man-switch
    • Code: 0x...

neps/nep-0480.md Outdated Show resolved Hide resolved

The following host functions are added to the NEAR host environment:

- `predecessor_namespace() -> Namespace`: Returns the namespace of the predecessor account, which may be the empty namespace. In the case that the predecessor is not a smart contract, this is the empty namespace.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... so we are using the empty namespace to represent two different values: empty namespace in the predecessor or no smart contract. Could this be a problem? Is there a good way to distinguish between the two?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we usually don't care too much whether a call comes from an EOA or contract, but if the distinction is pertinent, one may use env::signer_account_id().


Existing host functions will generally still refer to the account as a whole:

- `storage_usage() -> u64`: Returns the storage usage of the current account, including all namespaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current proposal, then there are two ways of getting storage used. storage_usage() and namespace_storage_usage(). I can see some developers getting confused when storage_usage() does not add up to what they are seeing the current namespace is actually using (they might have forgotten that other namespaces exist).

In the ideal world, it would be great to have a single hostfunction with a signature like: storage_usage(enum {EntireContract; Namespace(namespace)'}). Does something like this make sense?

If it does, I propose that we update the proposed namespace_storage_usage() to be something like the above and then mark storage_usage() as deprecated. We probably never will be able to remove it but hopefully with sufficient documentation, we will minimise the confusion.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I have enough information to have an informed opinion here. Making a decision one way or another has the potential to break contracts: if storage_usage() only references the current namespace, old contracts could incorrectly think the account is not using storage when it is, and if it references the whole account's usage (all namespaces), then old contracts could incorrectly think the current contract is using storage when it is not.

Co-authored-by: Akhilesh Singhania <akhi3030@gmail.com>
@frol frol added S-review/needs-sme-review A NEP in the REVIEW stage is waiting for Subject Matter Expert review. and removed S-review/needs-wg-to-assign-sme A NEP that needs working group to assign two SMEs. labels Jun 20, 2023
@walnut-the-cat
Copy link
Contributor

Hello- NEP moderator here.

@encody , is it still ongoing project?

@encody
Copy link
Author

encody commented Nov 23, 2023

@walnut-the-cat This project is not currently undergoing active development.

@walnut-the-cat
Copy link
Contributor

Thanks @encody. For now, I will mark it as retracted. If the project gets picked up again, please ping and we can re-open.

@walnut-the-cat walnut-the-cat added S-retracted A NEP that was retracted by the author or had no activity for over two months. and removed S-review/needs-sme-review A NEP in the REVIEW stage is waiting for Subject Matter Expert review. labels Nov 27, 2023
@flmel flmel closed this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NEP A NEAR Enhancement Proposal (NEP). S-retracted A NEP that was retracted by the author or had no activity for over two months. WG-protocol Protocol Standards Work Group should be accountable
Projects
Status: RETRACTED
Development

Successfully merging this pull request may close these issues.

8 participants