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

zero balance account NEP #448

Merged
merged 8 commits into from Feb 20, 2023
Merged

zero balance account NEP #448

merged 8 commits into from Feb 20, 2023

Conversation

bowenwang1996
Copy link
Collaborator

@bowenwang1996 bowenwang1996 commented Jan 11, 2023

NEP for #415. Implementation can be found in near/nearcore#8378

@render
Copy link

render bot commented Jan 11, 2023

neps/nep-0448.md Outdated Show resolved Hide resolved
neps/nep-0448.md Outdated Show resolved Hide resolved
@frol frol added WG-protocol Protocol Standards Work Group should be accountable S-draft/needs-author-revision A NEP in the DRAFT stage that needs an author revision. A-NEP A NEAR Enhancement Proposal (NEP). labels Jan 11, 2023
@bowenwang1996
Copy link
Collaborator Author

@frol @ori-near what are the next steps for this NEP?

@ori-near
Copy link
Contributor

Hi @bowenwang1996 – thank you for starting this proposal. If you believe this NEP is ready for review, please turn the PR into "Ready for review" state and ping the @near/nep-moderators. Once you do that, the moderators will check that the NEP meets all the requirements and will then request the Protocol Working Group to assign 2 technical reviewers.

@bowenwang1996 bowenwang1996 marked this pull request as ready for review January 18, 2023 20:28
@bowenwang1996 bowenwang1996 requested a review from a team as a code owner January 18, 2023 20:28
@ori-near ori-near added S-draft/needs-moderator-review A NEP in the DRAFT stage that needs a moderator review. and removed S-draft/needs-author-revision A NEP in the DRAFT stage that needs an author revision. labels Jan 18, 2023
@ori-near
Copy link
Contributor

Thank you @bowenwang1996 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). If you want to assign yourself, please mention that you are acting as the Technical Reviewers.

Please find some guidelines below for completing the technical review.

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 recommendation for the working group if the NEP is ready for voting (it could be approving or rejecting recommendation)
    • A summary of benefits (example)
    • A summary of concerns and blockers that were identified on the way and their status (some will be resolved, others dismissed, etc) (example)

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

@ori-near ori-near added S-review/needs-wg-to-assign-sme A NEP that needs working group to assign two SMEs. and removed S-draft/needs-moderator-review A NEP in the DRAFT stage that needs a moderator review. labels Jan 18, 2023
near-bulldozer bot pushed a commit to near/nearcore that referenced this pull request Jan 24, 2023
Implements near/NEPs#448. Note: this pull request does not update account creation costs. If the NEP is approved, the costs will be updated separately.
@mfornet
Copy link
Member

mfornet commented Jan 24, 2023

As a working group member, I'd like to nominate @mm-near and @DavidM-D (because his team is working on the onboarding experience) as Subject Matter Experts to review this NEP.

@ori-near ori-near 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 Jan 24, 2023
@ori-near
Copy link
Contributor

ori-near commented Jan 24, 2023

Hi @DavidM-D and @mm-near! Thank you for agreeing to review this NEP. Per the NEP workflow, the expectation is for you to complete the review within one week (Tuesday, January 31). Please find additional guidelines below.

Technical Review Guidelines
  • First, review the proposal. 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 recommendation for the working group if the NEP is ready for voting (it could be approving or rejecting recommendation)
    • A summary of benefits
    • A summary of concerns and blockers that were identified on the way and their status (some will be resolved, others dismissed, etc)
## Example

### 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.

@frol
Copy link
Collaborator

frol commented Feb 5, 2023

As the moderator, I would like to thank the author @bowenwang1996 for submitting this NEP extension, and @mm-near @DavidM-D @jakmeier and @near/wg-protocol work group members for your review. Based on the voting indications above, and support from @bowenwang1996 it seems like this proposal is close to a decision. We are therefore scheduling the third Protocol Work Group meeting next week, where this NEP can enter the final voting stage. Anyone can discuss the technical details by adding your comments to this discussion. And join the call to learn about the final decision and how to get more involved.

Meeting Info
📅 Date: Thursday, Feb 9, 4 PM UTC
✏️ Register

@mfornet
Copy link
Member

mfornet commented Feb 6, 2023

As a working group member, I lean towards approving this proposal.

@k3y0k3y
Copy link

k3y0k3y commented Feb 6, 2023

I am in support of this proposal. Agree with the feedback that a UI design to indicate to a user when they are reaching key-limit and need NEAR to continue is needed. Also agree with the feedback that in documentation of using this service there should be clear indication to use it in limited use cases where meta-tx can cover the costs or show a clear on-ramp for earning NEAR.

@bowenwang1996
Copy link
Collaborator Author

Responding to a few concerns mentioned by various people:

  • The number of access keys allowed is too small. @DavidM-D suggested having four full access keys instead of 2. This is actually something we could consider changing, as adding one more access key only adds 82 bytes to storage. @jakmeier do you know how much more can we increase account creation cost before something else breaks?
  • No incentive for people to delete zero balance accounts. In general we observed very little deletion of data, let alone accounts. It doesn't appear that people are really incentivized to delete their account even with storage staking.
  • The experience of "graduating" from a zero balance account is suboptimal as a user suddenly have to pay for the storage of all their keys. I'd argue that here the impact on UX is quite minimal since in our onboarding experience, a user graduates from a zero balance account either because they've earned NEAR, or because they decide to fund their account. In either case, paying for an additional <1000 bytes worth of storage (0.01N) doesn't seem to be too much of a problem.
  • Increase of account creation cost may break some contracts. Thanks to the awesome work done by @jakmeier, we now know that only the contract on near needs to be upgraded for this change.
  • Account creation throughput is lower due to increased cost. I don't think the throughput here is a big concern. Today with 4 shards we can create ~668 accounts per block and we plan to have even more shards in the future.
  • Gas now can be burnt for storage (vs. before it is just about compute). Yes this is a conscious change and it feels to me that this is in the right direction of how storage should work. As mentioned in [Discussion] Removing storage staking for base account information #415, storage staking has a number of issues including the difficulty of storage management and bad ux for end users. In addition, people do not really delete data from storage anyways. We should think in general how storage management works, as suggested in Rethink storage management #185

@jakmeier
Copy link
Contributor

jakmeier commented Feb 7, 2023

This is actually something we could consider changing, as adding one more access key only adds 82 bytes to storage. @jakmeier do you know how much more can we increase account creation cost before something else breaks?

82 extra bytes, I assume the cost would still be < 7 Tgas, right? That seems completely fine.

If the total gas goes above 9Tgas, we have to look into the-auction-factory.near. But I think we could even go up to 20 Tgas and only need to quickly check 4 contracts that will probably turn out to be no problem anyway, as most of them allow to attach more gas by users to resolve the problem.

@bowenwang1996
Copy link
Collaborator Author

82 extra bytes, I assume the cost would still be < 7 Tgas, right? That seems completely fine.

If we want to have 4 full access keys, which is 2 more than the current design, then we need to add 164 bytes, which brings the account creation cost to 7.6Tgas

@jakmeier
Copy link
Contributor

jakmeier commented Feb 8, 2023

Oh, I see. 7.6 Tgas is also fine though, I just had to do some more checks.

Details

Only the-auction-factory.near falls in the problematic range and it seems all offending calls are to create_store, when it has only 100 Tgas attached. It will forward too much to on_create and not have enough to cover the extra account creation cost.
Luckily, this is a direct call and users can just attach 110 Tgas instead and it will work again.
Example: HtmRj7qbGofHmANWt9A6E2SpNjfzAWeQw6ZjxNJvSkmD

@frol frol added S-approved A NEP that was approved by a working group. and removed S-voting/needs-wg-voting-indication A NEP in the VOTING stage that needs the working group voting indication. labels Feb 10, 2023
@frol
Copy link
Collaborator

frol commented Feb 10, 2023

Thank you to everyone who attended the third NEAR Protocol Working Group meeting! The working group members reviewed the NEP and reached the following consensus:

Status: Approved

Meeting Recording:
https://youtu.be/ktgWXjNTU_A

@bowenwang1996 Thank you for authoring this NEP!

Next Steps:

Implementation Status: Implemented in nearcore, but not yet stabilized

@bowenwang1996
Copy link
Collaborator Author

Update: after a discussion with @k3y0k3y we decided to change the number of full access keys allowed to 4. This increases the account creation cost to 7.6Tgas. As we see in the discussion above, it won't break more contracts.

@jakmeier
Copy link
Contributor

jakmeier commented Feb 14, 2023

Today I learned: Every NEAR token transfer action to an implicit account costs transfer cost + account creation cost + add full access key cost. Even if the account already exists.

It makes sense in hindsight, we don't know if the account exists when we send the transfer. And for some reason NEAR uses transfer actions to create implicit accounts instead of create account actions, which I was long aware of. I just didn't connect the dots for what it means for normal transfers.

This has two consequences for the zero balance account feature which I haven't realized previously. Maybe the SMEs did think of it already, but to make sure we are not glancing over it, here are the implications:

  • normal NEAR transfers to any implicit account address will now cost 0.23 Tgas + 0.2 Tgas + 7.6 Tgas = 8.03 Tgas
  • If there are any contracts that use transfers to implicit accounts inside a indirect function call, it is possible they break with this change. My checks would not have covered these cases. I don't know an easy way to check for these either.
    But tbh, I am not sure why this pattern would be used in the first place. It is not inherently clear to me if it's even worth worrying about such cases.

@bowenwang1996 @mm-near @DavidM-D
Any thoughts on this? Do you think we can proceed as planned or does this substantially change the assumptions under which this has been accepted?

@frol
Copy link
Collaborator

frol commented Feb 14, 2023

@jakmeier That is a really good call. Exchanges heavily rely on implicit accounts as they own implicit accounts for each user, and now users will pay higher fees to top-up their CEX account balances, and CEX might also use implicit accounts internally for their cold wallets.

near-bulldozer bot pushed a commit to near/nearcore that referenced this pull request Feb 16, 2023
# Feature to stabilize
Zero Balance Account that enables a more smooth onboarding experience where users don't have to first acquire NEAR tokens to pay for the storage of their accounts.

# Context

- NEP: near/NEPs#448
- Implementation: #8378

# Testing and QA
- unit tests in runtime/runtime/src/verifier.rs
- integration tests in integration-tests/src/tests/client/features/zero_balance_account.rs

# Checklist
- [x] [Link to nightly nayduck run](https://nayduck.near.org/#/run/2869)
- [x] Update CHANGELOG.md to include this protocol feature in the `Unreleased` section.
@bowenwang1996
Copy link
Collaborator Author

Updated the spec for zero balance account as per discussion here. @jakmeier I created #462 to document your concern, but as we discussed, it does not seem to be a blocker for this NEP. @firatNEAR we may want to check with partners who manage accounts on users' behalves and often do token transfers to implicit accounts.

@bowenwang1996
Copy link
Collaborator Author

@frol summary of benefits and concerns:

Benefits

  • Users can now onboard with having to acquire NEAR tokens
  • Together with meta transactions, this allows a user to start interacting with an app on NEAR directly from their device without any additional steps
  • Solves the problem of meta transaction for implicit accounts

Concerns

# Concern Resolution Status
1 The number of full access keys allowed is too small Could be done in a future iteration. Resolved
2 No incentive for people to remove zero balance account. Very few people actually delete their account anyways. Resolved
3 UX of requiring balance after a user graduate from zero balance account to a regular account The experience of graduating from zero balance account should be handled on the product side Resolved
4 Increase of account creation cost may break some existing contracts A thorough investigation has been done and it turns out that we only need to change the contract that is deployed on near slightly Resolved
5 Account creation speed is slower due to increased cost Unlikely to be a concern, especially given that the number of shards is expected to grow in the future Resolved
6 Cost of transfers to implicit account increases Unlikely to break anything at the moment, and could be addressed in the future in a different NEP (see #462 for more details) Resolved

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.

As a NEP moderator, I am approving this PR following the Protocol Work Group approval and the decision context incorporated into the NEP document.

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-approved A NEP that was approved by a working group. WG-protocol Protocol Standards Work Group should be accountable
Projects
Status: APPROVED NEPs
Development

Successfully merging this pull request may close these issues.

None yet

8 participants