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

proposal to restrict ethereum address on NEAR #492

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Conversation

bowenwang1996
Copy link
Collaborator

@bowenwang1996 bowenwang1996 commented Jul 27, 2023

Proposal to restrict the creation of Ethereum addresses on NEAR to prevent loss of funds and enable future possibilities for Ethereum wallets to support NEAR transactions.

Implementation: near/nearcore#9365

@render
Copy link

render bot commented Jul 27, 2023

@pvolnov
Copy link

pvolnov commented Jul 27, 2023

What restrictions are there on creating an address through the registrar account? Will it require a signature with the Ethereum private key to prevent spam?

@abacabadabacaba
Copy link
Collaborator

Seems hacky. Why not restrict creation of top-level accounts entirely? Is there a use case for user-created top-level accounts (other than implicit accounts)?

Making a wallet that automatically switches between NEAR and Ethereum based on address doesn't sound good, because this approach cannot be extended to support other chains (many of which use the same address format as Ethereum). Similarly, this proposal may prevent users from accidentally withdrawing their tokens to NEAR, but not to any of the other chains, so this solution doesn't scale.

By the way, do exchanges even prevent withdrawals to a non-existent address? In most blockchains, transferring tokens to a non-existent address is not a problem, so many exchanges may not have any code to detect that.

A much better approach would be to standardize a set of prefixes denoting the chain, e.g. prepend near: to every NEAR address, eth: to every Ethereum address, and so on. If the wallets refuse to send tokens to an address without a correct prefix, the possibility of making a mistake would be greatly reduced, and this works with all chains.

@mfornet
Copy link
Member

mfornet commented Jul 28, 2023

This seems to be a problem to be solved in the wallet / web interface, rather than in the protocol. Agree with @abacabadabacaba that this proposal feels hacky. What if someone pastes an ethereum address without 0x prefix, that could be a source of confusion as well. We can restrict altogether the creation of TLD without going through registrar, and have this logic on the registrar itself, rather than in the protocol.

neps/nep-0492.md Outdated
Comment on lines 68 to 70
### Backwards Compatibility

There is no backwards compatibility concern.
Copy link
Member

Choose a reason for hiding this comment

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

To some extent there is given there are ~5k such addresses already.

@frol frol added WG-protocol Protocol Standards Work Group should be accountable S-draft/needs-moderator-review A NEP in the DRAFT stage that needs a moderator review. A-NEP A NEAR Enhancement Proposal (NEP). labels Jul 30, 2023
@frol
Copy link
Collaborator

frol commented Jul 30, 2023

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

@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-moderator-review A NEP in the DRAFT stage that needs a moderator review. labels Jul 30, 2023
@bowenwang1996
Copy link
Collaborator Author

yeah I think restricting all TLDs (other than implicit accounts) make sense.

@bowenwang1996
Copy link
Collaborator Author

@abacabadabacaba @mfornet the proposal is updated to restrict the creation of all top level accounts (other than implicit accounts)

Copy link
Contributor

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

Seems ok to me as a way to prevent scams. Might be worth doing a little more research though to see if there are other top-level accounts which currently exist and would have been prevented by this proposal.


In addition to prevent loss of funds for users, this change allows the possibility of Ethereum wallets supporting NEAR transactions, which could enable much more adoption of NEAR. The exact details of how that would be done is outside the scope of this proposal.

There are currently ~5000 Ethereum addresses already created on NEAR. It is also outside the scope of this proposal to discuss what to do with them.
Copy link
Contributor

Choose a reason for hiding this comment

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

What about other non-Ethereum-like top level accounts whos creation would have been restricted by this proposal?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are a total of 11423 accounts on mainnet that are top-level accounts longer than 32 characters and that are not implicit accounts. I don't think there is a clear pattern that people use top-level accounts longer than 32 characters for.

@frol frol added S-voting/needs-wg-voting-indication A NEP in the VOTING stage that needs the working group voting indication. and removed S-review/needs-wg-to-assign-sme A NEP that needs working group to assign two SMEs. labels Jul 31, 2023
@bowenwang1996
Copy link
Collaborator Author

@frol given that both Marcelo and Michael have approved the proposal, looks like we have a majority of support from the protocol working group. What else do we need to merge this proposal?

@frol frol added S-voting/final-comment-period A NEP in the final REVIEW stage. Last chance to bring up concerns before the voting call. and removed S-voting/needs-wg-voting-indication A NEP in the VOTING stage that needs the working group voting indication. labels Aug 25, 2023
@frol
Copy link
Collaborator

frol commented Aug 25, 2023

@bowenwang1996 Thank you for reaching out! I moved this NEP to final comment period and will ask @near/nep-moderators to schedule a Protocol WG call to present this NEP to community and formally vote on it.

Copy link
Contributor

@birchmd birchmd 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 member of the protocol working group, I approve this NEP.

Some notes from the discussion we had at the last open working group meeting:

  • The title should be changed to reflect the increased scope (all top level accounts disallowed, not just Ethereum-like)
  • Concerns around censorship should be addressed in follow-up discussions (i.e. actually spec out the top-level account name auction system; and in the meantime make the current registrar account more permissive).

@mfornet mfornet self-requested a review September 1, 2023 16:40
Copy link
Member

@mfornet mfornet 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 member of the Protocol WG I approve this NEP

@ori-near ori-near added S-approved A NEP that was approved by a working group. and removed S-voting/final-comment-period A NEP in the final REVIEW stage. Last chance to bring up concerns before the voting call. labels Sep 2, 2023
@ori-near
Copy link
Contributor

ori-near commented Sep 2, 2023

Thank you to everyone who attended the fifth Protocol Work Group call yesterday! The work group members reviewed the NEP and reached the following consensus:

Status: Approved

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

@bowenwang1996 Thank you for authoring this NEP!

Next Steps:

github-merge-queue bot pushed a commit to near/nearcore that referenced this pull request Sep 26, 2023
github-merge-queue bot pushed a commit to near/nearcore that referenced this pull request Oct 2, 2023
nikurt pushed a commit to near/nearcore that referenced this pull request Oct 3, 2023
github-merge-queue bot pushed a commit to near/nearcore that referenced this pull request Oct 10, 2023
Stabilize near/NEPs#492. Restrict the creation
of non-implicit top-level account that are longer than 32 bytes. Only
the registrar account can create them. More context on the proposal:

- The NEP has been officially approved in September
(near/NEPs#492 (comment)).
- #9589 implements the feature and includes tests checking that TLA can
no longer be created in the new protocol version.
nikurt pushed a commit to near/nearcore that referenced this pull request Oct 11, 2023
nikurt pushed a commit to near/nearcore that referenced this pull request Oct 11, 2023
Stabilize near/NEPs#492. Restrict the creation
of non-implicit top-level account that are longer than 32 bytes. Only
the registrar account can create them. More context on the proposal:

- The NEP has been officially approved in September
(near/NEPs#492 (comment)).
- #9589 implements the feature and includes tests checking that TLA can
no longer be created in the new protocol version.
nikurt pushed a commit to near/nearcore that referenced this pull request Oct 11, 2023
neps/nep-0492.md Outdated Show resolved Hide resolved
@frol frol merged commit a5bb580 into master Nov 16, 2023
4 checks passed
@frol frol deleted the restrict-ethereum-address branch November 16, 2023 19:51
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.

7 participants