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-518] Web3-Compatible Wallets Support #555

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

birchmd
Copy link
Contributor

@birchmd birchmd commented Jul 22, 2024

This PR contains the description originally posted in #518

--

NEP Status (Updated by NEP Moderators)

Status: Review

SME reviews:

@birchmd birchmd requested a review from a team as a code owner July 22, 2024 18:01
@flmel flmel added WG-protocol Protocol Standards Work Group should be accountable A-NEP A NEAR Enhancement Proposal (NEP). S-review/needs-sme-review A NEP in the REVIEW stage is waiting for Subject Matter Expert review. labels Jul 23, 2024
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.

Did a first read-through. Maybe its just on me for not being familiar with ETH, that I had to re-read many parts of the text multiple times over to get any semblance of comprehension, so bear with me…

Regardless I think there are some changes that can be made to the NEP text so that it is more approachable to the more casual readers like myself:

  1. Add a single glossary section where you'd define all terms and abbreviations and make sure that every term with a remote chance of ambiguity would be defined there;
  2. Speaking of terms, it would help comprehension if the terms were disambiguated a little more, especially when they refer to specific fields within an ETH-formatted transaction vs. a NEAR-formatted transaction. So for instance all ETH-specific terms could be prefixed with Eth or whatever. Otherwise readers are forced to try and infer the meaning of many words in the text either from their (possibly spotty) knowledge or from the context. (Although not mentioned in the text, I at some point realized that capitalized terms generally refer to ETH transaction fields, whereas the lowercase ones would be from the NEAR-land. But then some ETH functions are lowercase and NEAR actions are upper-cased...)
  3. Many of the "what" would benefit from substantiation by a "why" even if by a link to text elsewhere in the NEP or the broader internet that would explain the reasoning or logic. This was especially apparent in the latter sections describing the behaviour of separate components (what is ethereum decimal count that's forcing RPC to truncate? Why does RPC always estimate 30M? Why are self-transactions special-cased?)
  4. As I was reading this, I felt that large chunks of the NEP text are not at all a part of the NEAR Protocol and rather described either how a component remotely related to a Protocol feature would work or be implemented. If nothing else it would be great to clearly delineate what parts are actually protocol things, and which ones are supporting background information.

neps/nep-0518.md Outdated

#### Ethereum Transaction Construction

- Set the `chain ID` to a public constant (equal to [397](https://chainlist.org/chain/397) for mainnet and [398](https://chainlist.org/chain/398) for testnet).
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 public constants forever set in stone? From the sound of it they are, in which case the constants likely deserve more attention than a parenthesized side note. What a parenthesized side note could have, on the other hand, is where exactly the store lies. At a first blush chainlist.org seems like a random community-driven project, rather than something very authoritative. If that's indeed the case I'm curious what would happen if somebody made a competing website to chainlist.org and assigned the same ID number to a different network. And if there is indeed an authoritative source that chainlist.org draws from, then we should be linking to that source.

neps/nep-0518.md Outdated

The `To` address and `data field` creation vary based on the dApp's input to the Wallet Selector:

- **[COMPLEX]** If the `receiverID` matches `^0x[a-f0-9]{40}$` (indicating another Ethereum account), it's treated as an EOA and used as-is. This case is restricted to $NEAR transfers (excluding smart contract calls) and is integral to the **Ethereum standards simulation**.
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: This is the first mention of receiverID. Context suggests this refers to a field in a NEAR transaction.

neps/nep-0518.md Outdated
- The `Function Selector`,
- Function call parameters.

Both `To` and `From` addresses, encoded in hexadecimal and prefixed with “0x”, comprise 20 bytes. The `From` address is the right-most 20 bytes of the **Keccak-256** hash of the binary public key of the transaction sender (an **EOA**, externally owned address). The To address signifies either the recipient EOA or an on-chain contract address.
Copy link
Contributor

Choose a reason for hiding this comment

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

Glossary section for all TLAs (three letter acronyms) and other terms that aren't otherwise very clearly defined would be helpful, so that readers did not need to grep for the definition in the text.

neps/nep-0518.md Outdated
The `To` address and `data field` creation vary based on the dApp's input to the Wallet Selector:

- **[COMPLEX]** If the `receiverID` matches `^0x[a-f0-9]{40}$` (indicating another Ethereum account), it's treated as an EOA and used as-is. This case is restricted to $NEAR transfers (excluding smart contract calls) and is integral to the **Ethereum standards simulation**.
- Note: **NEAR protocol will not host Ethereum smart contracts on Ethereum-style accounts** post-implementation of this proposal.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wording nit: the current wording suggests that we're closing the door to any future NEP that would enable ethereum smart contracts hosted on NEAR protocol once this NEP is accepted. Is that intended?

neps/nep-0518.md Outdated
- etc.
- **[COMPLEX]** The `yoctoNear` values are 32-bit, accommodating values under `1e6`. The total `yoctoNEAR` attached to a NEAR Action is `1e6 * <ethereum transaction value> + <yoctoNEAR in data field>`, enabling:
- Accurate display of $NEAR balances in Ethereum wallets (assuming 18 decimals).
- Compatibility with NEAR standards like NEP-141, requiring 1 yoctoNEAR for ft_transfer and ft_transfer_call methods.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the implication that for these method call transactions the <ethereum transaction value> would be 0? I guess I should also ask what is <ethereum transaction value>, exactly?

neps/nep-0518.md Outdated

6. **`Nonce` Verification and Update**: It ensures the transaction nonce matches the stored nonce and then increments the stored nonce by one.

7. **[COMPLEX] Value Calculation**: The contract sets the `Value` by multiplying the Ethereum transaction value by 1e6 and adding the `yoctoNear` value from the data field. It then confirms the attached deposit is either greater than or equal to this value or identifies it as a self-transaction (refer to **TR and Gas Payment** section for details).
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to introduce the self-transaction exception here? Do the regular NEAR token transfers succeed if an attempt to transfer more NEAR to self is made? More generally this NEP talks quite a lot about WHAT certain components do, but rarely provide any motivation as to WHY behaviour is suggested to be that specific way.

neps/nep-0518.md Outdated

#### Approaches to Gas Payment

This project's challenge lies in appropriately assigning gas costs to the Ethereum-compatible account. Our proposed solutions are:
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for writing down options and suggestions in a NEP? Shouldn't NEP be a normative document that mandates specific behaviour? If the implementations are allowed to be flexible for whatever reason, then I'd argue that NEP is not the right place to list possible implementation approaches.

neps/nep-0518.md Outdated

## Scope of the Project

The scope of this project is extensive, encompassing several critical components:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: NEP is probably not the best place for project sizing and tracking. One thing to keep in mind is that nearcore is just an implementation of the NEAR Protocol, and NEPs are meant to describe the changes to the protocol, rather than a specific implementation of it.

neps/nep-0518.md Outdated

4. **Transaction Relayer (TR)**: Ethereum wallets cannot generate NEAR-compatible transactions. Instead, Ethereum-compatible transactions produced by them are processed by the Transaction Relayer, which embeds it into a NEAR transaction and forwards it to the user’s Wallet Contract. Again, while operated in conjunction with the RPC-Translator, the Transaction Relayer is distinct in its role.

5. **[COMPLEX] Ethereum Translation Contract (ETC)**: Addressing the incompatibility between NEAR's human-readable account names and Ethereum's cryptographic hash-based addresses, the ETC functions as an on-chain mapping system. This system records NEAR-compatible input values (like NEAR account names and smart contract function names) and maps them to their corresponding Ethereum-compatible cryptographic hashes. This feature is vital for preserving familiar user experiences, such as recognizing ft_transfer operations in NEP-141 as fungible token (ERC20) transfers, rather than generic contract calls, and ensuring that fungible token balances are displayed in Web3 wallets.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're in the normative section of the NEP here, is the list of translations a part of the NEAR protocol? Is the ETC itself a part of the NEAR protocol? More generally "such as", "for instance" and similar wordings read as somewhat suspect in the normative section of the protocol specification.

neps/nep-0518.md Outdated
- The `Function Selector`,
- Function call parameters.

Both `To` and `From` addresses, encoded in hexadecimal and prefixed with “0x”, comprise 20 bytes. The `From` address is the right-most 20 bytes of the **Keccak-256** hash of the binary public key of the transaction sender (an **EOA**, externally owned address). The To address signifies either the recipient EOA or an on-chain contract address.
Copy link
Contributor

Choose a reason for hiding this comment

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

Side note: I don't see #518 (comment) having been addressed in the NEP (collection of NEPs should be in some ways as self-contained as feasible; keeping motivation as a comment somewhere is not discoverable to most readers).

@birchmd
Copy link
Contributor Author

birchmd commented Jul 23, 2024

Thanks for the detailed review @nagisa ! I agree the idea proposal didn't end up working all that well as an NEP after all. I'll spend some time giving it a pretty significant re-write. To confirm that I understand your comments correctly, are you are suggesting the following changes?

  1. Write the document from a Near perspective, defining the EVM concepts clearly and assuming the reader is no familiar with them.
  2. Remove (perhaps replace with a link to the original 518 issue) much of the descriptions of components that are not direct protocol changes (i.e. RPC, wallet selector).
  3. Ensure remaining descriptions (of the wallet contract and protocol-level handling of eth-implicit accounts) are specifications with precise descriptions of behavior (though the reasons for this behavior can still be motivated by higher level discussions of how the wallet contract fits into the broader solution).

Let me know if I have misunderstood the changes you would like to see.

@nagisa
Copy link
Contributor

nagisa commented Jul 24, 2024

Write the document from a Near perspective, defining the EVM concepts clearly and assuming the reader is no familiar with them.

Yes, please.

Remove (perhaps replace with a link to the original 518 issue) much of the descriptions of components that are not direct protocol changes (i.e. RPC, wallet selector).

Yes.

Ensure remaining descriptions (of the wallet contract and protocol-level handling of eth-implicit accounts) are specifications with precise descriptions of behavior (though the reasons for this behavior can still be motivated by higher level discussions of how the wallet contract fits into the broader solution).

Sure, so long as those higher level discussions are either part of this NEP or are referenced here in some way or form.

@birchmd
Copy link
Contributor Author

birchmd commented Jul 24, 2024

@nagisa I started doing the rewrite. Some sections are still left with TODO, but I wanted to give you a chance to take a look at what I have so far! I'll work on filling in the remaining sections if you think the style and structure of the rewrite looks good.

@akhi3030
Copy link
Contributor

Thank for you for the initial review @nagisa. I agree with your assessment and that a rewrite is needed.

Thank you for starting the rewrite @birchmd. To set expectations, I will be on PTO all of next week. So if the NEP is not quite ready to reviewed before EOW, then my review will be delayed. I hope the timeline works fine?

@birchmd
Copy link
Contributor Author

birchmd commented Jul 24, 2024

@akhi3030 thanks for the heads up. I probably will not have time to finish this by EOW. But each section is reasonably self-contained, so I encourage you to review what I have already written so far this week. Then I can address your comments and you can review those changes along with the rest of the new sections after you return from PTO.

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.

incomplete review... just covered a small section.

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

### Eth-implicit accounts

**Definition**: An eth-implicit account is a top-level Near account with ID of the form `^0x[a-f0-9]{40}$` (i.e. 42-characters with `0x` as a prefix followed by 40 characters of hex-encoded data which represents a 20-byte address).
Copy link
Contributor

Choose a reason for hiding this comment

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

can this scheme be extended in future if we need to add support for different types of account ids?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not clear how this could be extended to support other types of account IDs.

During the design we thought about having a chain-specifying prefix (e.g. eth), or having the addresses be sub-accounts of a chain-specifying account (again e.g. eth), but we ultimately chose to just have the account ID be equal to the address. The reasons for this choice are (1) with the number of EVM chains that exist now it doesn't really make sense to have a chain specifier with an address because they exact same key could control the same address on multiple chains (e.g. Etheruem, Aurora, Arbitrum, BNB, etc) -- including now the Near chain with this proposal; (2) it simplifies how users can look up their accounts in Near explorers (they just search for their address exactly as they would in etherscan for example, though with the caveat that they cannot use the address checksum casing because Near requires account IDs to be lowercase).

@birchmd
Copy link
Contributor Author

birchmd commented Aug 6, 2024

@nagisa @akhi3030 This PR is ready for further review

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.

Another incomplete review. Will try to get through more of it later this week.

This is accomplished through two key protocol changes:

1. Ethereum-like addresses (i.e. account IDs of the form `^0x[a-f0-9]{40}$`) are implicit accounts on Near (i.e. can be created via a `Transfer` action). We call these "eth-implicit accounts".
2. Unlike the current implicit accounts (64-character hex-encoded), eth-implicit accounts do not have any access keys added to them on creation. Instead, these accounts will have a special contract deployed to them automatically called the "wallet contract". This wallet contract enables the owner of the Ethereum address corresponding to the eth-implicit account ID to sign transactions with their Ethereum private key, thus providing similar functionality to the default access key of 64-character implicit accounts.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible in future for me to "deploy" a different contract on my ETH implicit account? Could there be interesting use cases for such a feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it is impossible to deploy a different contract to a new eth implicit account. This could be enabled if the wallet contract was upgraded to allow users to take the DeployContract action or if the wallet contract allowed adding full access keys. However, this is a dangerous feature because it could result in users no longer being able to control their account using Ethereum-like transactions.

I cannot think of good use cases where the benefits of being able to deploy a different contract are worth the risk of a user accidentally locking themselves out of their account. But if there is one in the future then we can consider upgrading the wallet contract in another protocol change.

neps/nep-0518.md Outdated Show resolved Hide resolved
neps/nep-0518.md Outdated Show resolved Hide resolved
neps/nep-0518.md Outdated Show resolved Hide resolved
neps/nep-0518.md Outdated Show resolved Hide resolved
neps/nep-0518.md Outdated
- `get_nonce` is a view function which takes no arguments and returns a 64-bit number (encoded as a base-10 string).
- `rlp_execute` is the main entry point for executing user transactions. It takes two inputs (encoded as a JSON object): `target` is an account ID (i.e. string) which indicates the account that is supposed to be the target of the Near action; and `tx_bytes_b64` is a string which is the base-64 encoding of the raw bytes of an Ethereum-like transaction. The process by which a Near action is derived from the Ethereum transaction is described below.

The wallet contract has two state variables: the nonce, a 64-bit number; and a boolean flag indicating if a transaction is currently in progress. As with nonce values on Near access keys, the purpose of the wallet contract nonce is to prevent replaying the same Ethereum transaction more than once. The boolean flag prevents multiple transactions from being in-flight at the same time. The reason this is needed is because of the asynchronous nature of Near as compared with the synchronous nature of the **EVM**. On Ethereum if two transactions are sent (they must have sequential nonces per the Ethereum standard) then the all actions of the first will always happen before all the actions of the second. However, on Near there is no guarantee of the order of execution for receipts in different shards. Therefore, the only way to ensure that all actions from the first transaction are executed before all the actions of the second transaction is to prevent the second transaction from starting its execution until after the first one entirely finishes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's explained further below but leaving a comment for now in case I forget. How is the boolean flag used exactly? What happens when it indicates that a transaction is in flight? Users cannot control in which order their transactions are getting processed, will their contracts be rejected arbitrarily? Is it possible that a user has submited transactions 1,2,3. 1 is being processed so 2 gets rejected and then when 1 is processed, 3 arrives and gets processed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If rlp_execute is called while the boolean flag is set (so there is already a transaction in flight) then that rlp_execute call will immediately fail with an error about not allowing simultaneous transactions.

In your example where transaction 2 is rejected because 1 is still executed then 3 will also be rejected because the wallet contract follows the Ethereum standard of requiring nonces to be sequential. Transaction 3 will be rejected for having the wrong nonce until transaction 2 executes.

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-review/needs-sme-review A NEP in the REVIEW stage is waiting for Subject Matter Expert review. WG-protocol Protocol Standards Work Group should be accountable
Projects
Status: REVIEW
Development

Successfully merging this pull request may close these issues.

None yet

5 participants