So in general I’m massively in favor of the general idea of this proposal.
I’ll start with a description of the relevant fields from the current Phonon spec data structure to provide some context as to the differences between this proposal and what’s currently implemented. I’m describing the current master
branch of the phonon-client repo here, I suspect the phonon-network specification may be a little out of date.
We currently have the following:
CurrencyType: A uint16 identifier for the type of blockchain asset.
ChainID: An int identifier for the specific blockchain instance to contact
Denomination: The value in chain base units contained within this phonon
Current examples of CurrencyType are the following, from model/phonon.go
:
type CurrencyType uint16
const (
Unspecified CurrencyType = 0x0000
Bitcoin CurrencyType = 0x0001
Ethereum CurrencyType = 0x0002
EthereumERC721 CurrencyType = 0x0003
EthereumERC20 CurrencyType = 0x0004
)
So as compared to the proposal above this data structure basically comprises a combination of NetworkTypeId and AssetTypeID within one identifier. A uint16 gives us 65536 potential contract types across all blockchains that we could support.
The reason I like this as one field instead of two, is that this field is the data that must be used by the client to provide instructions on how to process the phonon during on chain interactions, e.g. deposit, validate, and redeem. Since ETH and ERC20 require distinct processing instructions, they would have distinct CurrencyTypes in this model.
I guess I could see grouping all contracts from a single chain type together, because then you could provide a class that handles that chain, sharing some common functionality even among distinct contract types, but in practice that can also be accomplished by assigning multiple CurrencyType’s to be handled by one ChainService. Either way, you have to have some internal logic and switching on ID to handle different contract types on the same chain, or contract types vs native.
In a nutshell, I’m not sure it’s worth incurring the cost of the code change on the card to split CurrencyType into two fields, because I don’t think it allows us to simplify the code any further. I’m willing to change my mind if there’s some benefit I’m not thinking of here.
However, I definitely like the name AssetType better than CurrencyType, so I am in favor of switching that name throughout the codebase. As hinchy mentioned, it describes the potential for NFTs and other types of assets much better.
On chainID, this basically provides an ID that can be used as a switch to use different endpoints in the ChainService implementation, I think we’re in total agreement on the necessity of this field. This was added along with the initial redeem functionality several weeks ago.
On ValueBase aka Denomination, this field is currently included in every phonon because it theoretically allows the card to process the value to do filtering as a performance optimization before returning results to the terminal. This isn’t currently implemented but could be important when there are large numbers of phonons on cards.
It is not ideal that these bytes can be wasted on valueless AssetTypes. In practice though, even ERC721 NFT phonons will have an ETH balance tied to the same key used to pay gas during contract calls, so I expect this not to go to waste very often, plus it’s only 2 bytes, so there are other places to optimize first.
Keeping the uint16 CurrencyType, now renamed AssetType, this makes the requirement for defining a new Phonon Asset the following, which is very similar to what was proposed above.
AssetType: A 2 byte ID used by the client for selecting an appropriate chainService when interacting with this phonon on chain.
AssetName: A human readable name for the asset
Tags: Data additional to the standard phonon fields needed for interacting with this asset type.
All phonons would also have the standard on card data fields including ChainID, PubKey, and Denomination.
A ChainService implementation for this new type would then have to implement parsing the extended tags, connecting to the correct endpoints for each chainID, and processing Deposit, Redeem, and Validate for this asset type.