sui icon indicating copy to clipboard operation
sui copied to clipboard

Add network identity information to the on-chain SystemObject

Open bmwill opened this issue 3 years ago • 13 comments
trafficstars

Today there is no identifying information present on-chain making it difficult to know if a particular network is a devnet, testnet, or mainnet. Adding some sort of network_id or chain_id to the SystemObject would enable SC developers or users to query the network to figure out the identity of the network.

In addition we should probably include some mechanism into our client APIs for querying the network/chain information from a running node so that it is clear the state they are serving is either from a testnet or from a mainnet.

bmwill avatar Apr 15 '22 18:04 bmwill

Seems like a protocol P0 for testnet, maybe P1 for devnet.

todd-mystenlabs avatar Apr 16 '22 19:04 todd-mystenlabs

Thanks Brandon! This is important to protect against replaying in case of a future network fork as well, not necessarily between existing different networks.

kchalkias avatar Apr 16 '22 22:04 kchalkias

@kchalkias Do we plan to do anything here for DevNet?

lxfind avatar May 02 '22 15:05 lxfind

PR in progress https://github.com/MystenLabs/sui/pull/2116

kchalkias avatar May 23 '22 15:05 kchalkias

@kchalkias Any updates on this one?

lxfind avatar Jun 11 '22 02:06 lxfind

@joyqvq can you please provide some updates on this issue?

longbowlu avatar Aug 25 '22 22:08 longbowlu

the revived PR was parked last week bc did not want to introduce yet another breaking change atm, we can first land https://github.com/MystenLabs/sui/pull/3687, then switch over all signing operations to point over to the IntentMessage and verification performs checks on chain id.

Timeline wise we can revive next week, wdyt @kchalkias ?

joyqvq avatar Aug 25 '22 22:08 joyqvq

Because of Sui's data model, I think an explicit network identifier in a transaction/intent should not be needed. If you have two Sui networks with different TransactionDigest::genesis()'s, all transactions are non-replayable by construction (modulo hash collisions). This is because every tx has at least one owned object input, and every owned object input contains a direct or indirect commitment to TransactionDigest::genesis() via its object digest.

sblackshear avatar Oct 18 '22 16:10 sblackshear

I think we will need some genesis ceremony refactoring to ensure that we use a different TransactionDigest::genesis for devnet vs each testnet wave vs mainnet, but nothing should be needed at the tx or intent level.

sblackshear avatar Oct 18 '22 16:10 sblackshear

It may still be worthwhile to introduce and include a network ID/chain id, inside of the system object. But you're correct that we probably don't need such a construct in every transaction.

bmwill avatar Oct 18 '22 16:10 bmwill

Agreed on putting this inside the system object--useful for SC programmers and for clients to query.

sblackshear avatar Oct 18 '22 16:10 sblackshear

@sblackshear I've updated the OP and the title of this issue to reflect that goal.

bmwill avatar Oct 18 '22 16:10 bmwill

Note that the concept of signing intents is the signer should ideally be agnostic on if some or all of the domain separators are already included in the signed data object, this should be defined by design at the intent layer. Putting the domain separators in a different object helps us having everything as prefix to what we sign, so we know this data is always aligned between objects of different types and networks and not serialized at random positions internally.

I agree that due to our special way of deriving objectID, the chainID is included via genesis()'s inherent reference, and this is implementation detail.

But we should note that signers do not only sign txs, they do sign proof of possession/assets, personal messages, and potential other payloads that are used as smart contract sig inputs (ie Oracles). All of the other uses of signature do not necessarily have the indirect reference to chainID. At the same time we need to define a standard way of signing to avoid all of the issues Ethereum and other faced in their first steps where they needed to define EIPs on how to distinguish between txs and other objects signing.

Atm the standard we agreed was to sign the tuple <Intent, Data> and the Intent includes all major domain separation factors, for which we believe the most important are the following 4: a) appID so we know if a signature with this key was for Sui, for another chain or protocol etc. b) version of our Intent scheme (to ensure forward non-collision) c) subappID you might have multiple instances/installations of an app (this is chain/network ID for Sui). We don't want users who reuse their keys among different Sui networks to accidentally collide and someone takes a signature from one chain and submits it masqueraded as targeting another other network. This should apply to all data types we could potentially sign, which is not only txs. d) scope the actual objectType you sign (transaction, proof of asset, proof of possession, personal message, in-contract signature, consensus-sig etc).

That said, the Intent is a struct with all of the above 4 fields. In practice, a Transaction is the most dominant/used signed dat type and if I understand correctly you want an exception from the rule and remove the subappID (chainID) from the Intent, because it's already implied/included in the tx (inherently as @sblackshear explained). However, all of the other data types do not have this luxury, and the chainID is not necessarily included there, so for them we still maintain the chainID.

IMO we have two options here:

  1. just keep one struct as it is today, with the chainID included even if it's theoretically duplicated info for the Sui tx type.
  2. consider the SuiTx as a distinct app, we assign a different appID and then we can define the internal format which will only include appID, version, scope, saving one byte of information. The side effect is now we have to maintain two Intent structs, one for everything else and a special one for txs.

And of course a 3rd option is to completely retire intent signing and improvise in the future. Someone can claim that yes intent signing is ideal for a modern blockchain, but not a priority + it introduces breaking changes, and thus we should drop that security feature completely.

kchalkias avatar Oct 19 '22 08:10 kchalkias