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

Adding implicit account creation #71

Merged
merged 4 commits into from
Sep 14, 2020
Merged

Conversation

ilblackdragon
Copy link
Member

@ilblackdragon ilblackdragon commented May 27, 2020

Reasoning

Currently NEAR requires explicitly to create account by someone else to have an identifier that can be shared with others. This is somewhat mitigated by linkdrop pattern, but has effects on some of the existing blockchain tooling (wallets, custody).

For example multiple custody partners were interested in this, because their current flow is to create key pair inside secure offline location and explicitly creating account would require them building additional flow for extracting this into hot wallet, confirming, etc.

Also in the future, even if we don't get to #53 "exchange standard", this would provide a way to withdraw funds.

We are already recommending to use hex(public_key) as an account id for cases when there is no current way for user to specify account ids or not interested in doing so.

But currently you can't just create a key pair, and then share hex(public_key) with whoever is interested in sending you funds - because they can also take over that account.

This proposal limits account creation to only allow to create accounts of length 64 if they have AccessKey which hex(public_key) == account_id.

Additional consideration, if we want to make even CreateAccount implicit.
For example if you have Transaction { receiver_id=hex(public_key), actions=[Transfer(100)]}, if such account doesn't exist, it creates it with FullAccessKey(hex2bytes(receiver_id)).

PS. This creates a small benefit in pseudo-anonimity, as removes trace between account owner and account creator (which usually will be centralized service that will have tracking / etc).

@render
Copy link

render bot commented May 27, 2020

@render
Copy link

render bot commented May 27, 2020

Your Render PR Server at https://nomicon-pr-71.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-br720k08atn36pbt1eeg.

@evgenykuzyakov
Copy link
Contributor

This can be enforced through a smart contract and doesn't have to be enforced through the protocol.

The smart contract has control over it subdomains. No one else can create an account under this subdomain. It can also enforce key creation

@ilblackdragon
Copy link
Member Author

ilblackdragon commented Aug 25, 2020

@evgenykuzyakov

We have a lot of places where implicit account creation can lead to dramatic simplification of the interaction:

  • Exchange transfers
  • EVM transfers
  • Bridge transfers / interactions

I agree that in all of these cases, extra contracts and logic can achieve the same goal. But it seems like expanding runtime to dramatically improve usability across multiple use cases is a worthy goal.

@ilblackdragon
Copy link
Member Author

Implementation concerns:

  • When someone transfers $NEAR to the len(account_id) = 64 - if it doesn't exist, it gets created with FullAccessKey of the hex::decode(account_id)
  • There is no other way to create accounts with len(account_id) = 64

@evgenykuzyakov
Copy link
Contributor

There are some important limitations:

  • You can't add another access key to the account in the same transaction. Otherwise you can hijack an account without having the private key for the public key. I suggest we make it an invalid transaction to have any other actions on the 64 length accounts. NOTE: It's not easy to implement with upgradability, because the receipt doesn't have protocol version. And it might become invalid on epoch boundary.
  • We only support ED25519 keys for 64 length.

@evgenykuzyakov
Copy link
Contributor

There is also an issue with the cost of such receipt. Should we assume the cost of a receipt to a 64-len account includes AccountCreate and AddFullAccessKey?
This also affect VMLogic because it tracks fees there and subtract gas.

@bowenwang1996
Copy link
Collaborator

@evgenykuzyakov while receipts do not have protocol version, we can trace it back to the transaction that originated the receipt and find the protocol version of the block that includes the transaction.

@evgenykuzyakov
Copy link
Contributor

while receipts do not have protocol version, we can trace it back to the transaction that originated the receipt and find the protocol version of the block that includes the transaction.

That's not easy and involves extra work with potential attacks later. With multi-sharded system you have to query it, but people don't have answer you.

@MaksymZavershynskyi
Copy link
Contributor

Discussed with @evgenykuzyakov during the meeting. Overall, this looks like a good idea. @ilblackdragon 's implementation concerns do not require drastic changes, special casing receipt handling for 64-byte accounts does not look like will produce second-order effects. Also, @evgenykuzyakov 's limitation should be carefully implemented to avoid the abuse. It will probably reduce the TPS for transfers that have 64-byte receiver by 1.5-2x since the fees need to account for access key creation.

We discussed with @evgenykuzyakov how we can add upgradability to the receipts. @bowenwang1996 , instead of doing tracing of the original transaction, we should roll out a change that adds protocol version to each receipt. @evgenykuzyakov and me discussed how it can be implemented.

@evgenykuzyakov
Copy link
Contributor

After thinking through this I suggest the following implementation including protocol upgrade:

  • Pass protocol version to verify_and_charge_tx and validate_tx.
  • Update fee charging logic for receipts/transactions if the receiver_id length is 64 characters. Make Transfer action fee include extra fees for the CreateAccount and AddFullAccessKey. Make this only for the receipt and transactions under the new protocol version
  • When processing a receipt on 64-char account on a new protocol version do the following:
    • If account doesn't exist. Only a transaction with one action is allowed on this account, and this action is Transfer. It will create an account and add a full access key with nonce 0. The new transfer fee covers it.
    • If account does exist, then any actions are allowed and it will act like a regular account.

Notes:

  • We don't need to pass receipt version yet.
  • We don't modify receipts
  • It's possible that someone will create a transaction in past protocol and process the receipt in a new protocol resulting in paying less fees for this transaction. This is a minor issue and will not change total supply a lot. We're talking about 1-2 Tgas.
  • This implementation is fairly simple and can be done and tested very fast. We just need to correctly update fees computation in VMLogic and in a helper util module.

evgenykuzyakov pushed a commit to near/nearcore that referenced this pull request Sep 2, 2020
This PR implements near/NEPs#71

It contains a bunch of boilerplate code to pass the protocol version to VM and tests.

Logical changes:
- Bumps protocol version to 35
- Post protocol version, when a transfer action is executed on the 64-length hex like account ID as a single action in a transaction, it creates a new account if the account doesn't exist and add the ED25519 public key from the account ID hex representation.
- If a CreateAccount action attempts to create 64-length hex like account, it fails.
- Refunds don't automatically create accounts, because refunds are free and we don't want some type of abuse. NOTE: Account deletion with beneficiary creates a refund, so it'll not create a new account.
- `TransferAction` fee cost on such 64-len hex accounts include `CreateAction` and `AddFullAccessKey` costs
- The fee is also updated when calling a promise.
- `VMLogic::new` change is backward compatible until we release a new `near-sdk` version. So it shouldn't break master after pushing it.
- Bump `near-vm-*` versions to `2.2.0`.

TODO:

- [x] Add integration test for transfer from promise
- [x] Add migration test for transfer to 64len
- [x] Uncomment `VMLogic::new` for `near-sdk` `MockedBlockchain`

## Test plan:
- CI
- Added integration test and migration test
mhalambek pushed a commit to near/nearcore that referenced this pull request Sep 7, 2020
This PR implements near/NEPs#71

It contains a bunch of boilerplate code to pass the protocol version to VM and tests.

Logical changes:
- Bumps protocol version to 35
- Post protocol version, when a transfer action is executed on the 64-length hex like account ID as a single action in a transaction, it creates a new account if the account doesn't exist and add the ED25519 public key from the account ID hex representation.
- If a CreateAccount action attempts to create 64-length hex like account, it fails.
- Refunds don't automatically create accounts, because refunds are free and we don't want some type of abuse. NOTE: Account deletion with beneficiary creates a refund, so it'll not create a new account.
- `TransferAction` fee cost on such 64-len hex accounts include `CreateAction` and `AddFullAccessKey` costs
- The fee is also updated when calling a promise.
- `VMLogic::new` change is backward compatible until we release a new `near-sdk` version. So it shouldn't break master after pushing it.
- Bump `near-vm-*` versions to `2.2.0`.

TODO:

- [x] Add integration test for transfer from promise
- [x] Add migration test for transfer to 64len
- [x] Uncomment `VMLogic::new` for `near-sdk` `MockedBlockchain`

## Test plan:
- CI
- Added integration test and migration test
chefsale pushed a commit to near/nearcore that referenced this pull request Sep 7, 2020
This PR implements near/NEPs#71

It contains a bunch of boilerplate code to pass the protocol version to VM and tests.

Logical changes:
- Bumps protocol version to 35
- Post protocol version, when a transfer action is executed on the 64-length hex like account ID as a single action in a transaction, it creates a new account if the account doesn't exist and add the ED25519 public key from the account ID hex representation.
- If a CreateAccount action attempts to create 64-length hex like account, it fails.
- Refunds don't automatically create accounts, because refunds are free and we don't want some type of abuse. NOTE: Account deletion with beneficiary creates a refund, so it'll not create a new account.
- `TransferAction` fee cost on such 64-len hex accounts include `CreateAction` and `AddFullAccessKey` costs
- The fee is also updated when calling a promise.
- `VMLogic::new` change is backward compatible until we release a new `near-sdk` version. So it shouldn't break master after pushing it.
- Bump `near-vm-*` versions to `2.2.0`.

TODO:

- [x] Add integration test for transfer from promise
- [x] Add migration test for transfer to 64len
- [x] Uncomment `VMLogic::new` for `near-sdk` `MockedBlockchain`

## Test plan:
- CI
- Added integration test and migration test
@ilblackdragon
Copy link
Member Author

@evgenykuzyakov when you have a minute can you update this PR with the current design and merge this? So we have nomicon up to date.

specs/DataStructures/Account.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants