Issue with access key nonce

Development
1

Background

Currently we have the following issue with access key nonce: if someone deletes a key and adds the same key back with nonce 0, then old transactions signed with that key can potentially be submitted and processed again, which may cause trouble for the end users.

Solutions

There are in general two different categories of solutions, representing different views of the problem.

Address the issue on the tooling level

The reasoning here is that users are not supposed to delete a key and add the same key back. This is a practice that can shoot themselves in the foot and must be avoided. Consequently, to prevent users from falling to this trap, client sdks should take care of the issue so that developers don’t have to worry about it. For example, we could always use current_block_height * 1e6 as a nonce for newly created access keys, or simply use block timestamp. In the context of near-api-js, this would mean changing the following two functions to take nonce as an argument

However, one issue with this approach, besides being less reliable, is that this does not prevent duplicate transactions from being included onchain, which, while on its own is fine, causes trouble for rpc and indexer since transaction hash can no longer uniquely identify a transaction.

Address the issue on the protocol level

There are multiple ways to address this issue completely on the protocol level. One way that seems to work well is to require the access key nonce to be h * 1e6 where h is the block in which the add key transaction is included and for every transaction signed with the access key afterwards, they must have a nonce no larger than h * 1e6, where h is the block in which the transaction is included.

The disadvantage with this approach, besides needing to modify the protocol, is that we impose a range restriction on the access key nonces and as a result, it is not feasible if someone wants to create a key locally, sign some transactions with the key and then submit the add key transaction together with the transactions signed by the key. Note that we are not aware of anyone (including custody provider) who is doing this currently, so it might not be an actual issue.

4 Likes
2

I didn’t realize until this conversation that the tools were hardcoding the 0 into the nonce for an access key’s creation. To those who are curious, here’s where it’s at in near-api-js, lines highlighted and pinned to the latest commit at the time of this post:

1 Like
3

Some data points from Indexer infrastructure based on @alexatnear questions:

  • Max nonce that is currently observed on mainnet is 966780, and on testnet it is 247105
  • On mainnet there is no single ADD_KEY receipt action that uses non-zero nonce
  • On testnet there are 4 (four) receipts that set a non-zero nonce (the range is from 1 to 74)
1 Like
4

I suspect on testnet these come from @mattlockyer testing workaround for the issue

5

Unclear based on what @frol posted if that was from me.

6

Nope, those are not your transactions; those are related to my near-cli-rs testing. Did you test that 2FA flow fix on testnet or mainnet? I wonder why I don’t see them. In fact, what I think is 2FA flow still adds keys with zero nonce: NEAR Explorer | Transaction (it is a recent transaction)

7

Addressing issue on the protocol level is hard, but feasible. Addressing any issue on the tooling side is impossible since tools will evolve and new ones will get created.

In order to “fix” it on tooling side, we will need to BREAK all the tooling requiring to specify the block hash/height whenever you want to know the transaction status. You may argue that the tooling is already broken, but I would say that only the tooling that absolutely requires to handle every single transaction is broken. Even Explorer is not that sensitive (well, I know it is bad that we cannot display the transaction properly, but do you really care about that one glitch transaction right now?).

I believe that if you ask partners if they would prefer (1) a breaking API change (which will require extra data to pass, so requiring them to store block hash/height along with the transaction itself) OR (2) a fix on protocol level with potentially a hard-fork, most of them will chose the latter.

Using Explorer as a case study, as a user I want to paste the transaction hash and be done with that. If we have two transactions to display, we may show two search results in the drop-down for selection, but overall, I believe it would be quite confusing.

Is there a blockchain which tolerates transaction hash collisions on the protocol level? Should we define “transaction id” as something like block_hash:account_id:transaction_hash?

1 Like
8

After discussing with @frol @alexatnear @evgenykuzyakov we decide to move forward with a protocol level fix, which is submitted here.

2 Likes
9

I think this should’ve been NEP.

10

Yes, but we also want to get this change in quickly to avoid further issues (duplicate transactions) that we have to handle on the side of indexer.

11

So it feels like we can remove the explicit setting of nonce: 0 now? This might confuse future developers. Wondering if the protocol is expecting this key or not, or if it’s backwards compatible @Bowen

12

Yes we probably should do that to make it less confusing. The field still exists though.