dex/networks/{eth,erc20}: common token stuff #1509
Conversation
dex/networks/eth/tokens.go
Outdated
| NetTokens map[dex.Network]*NetToken `json:"netAddrs"` | ||
| // EVMFactor allows for arbitrary ERC20 decimals. For an ERC20 contract, | ||
| // the relation | ||
| // UnitInfo.Conventional.ConversionFactor * Token.EVMFactor = decimals |
There was a problem hiding this comment.
I see that the decimals variable for USDT and USDC is set to 6.
https://etherscan.io/address/0xdac17f958d2ee523a2206206994597c13d831ec7#readContract
https://etherscan.io/address/0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48#readProxyContract
Does this mean that if EVM factor is 9, then people will only be able to trade these in increments of $1000?
Also, I don't understand this comment: UnitInfo.Conventional.ConversionFactor * Token.EVMFactor = decimals. Isn't ConversionFactor = 10^EVMFactor?
There was a problem hiding this comment.
I see. For USDC/T, I'd want to give the full precision to the user, so I'd set UnitInfo.Conventional.ConversionFactor = 1e6 and an EVMFactor = 0, which I guess means that my use of a default value won't work. Darn.
That equation should read instead
math.Log10(UnitInfo.Conventional.ConversionFactor) + Token.EVMFactor = decimals
decimals is like the orders of magnitude that must be spanned from the EVM units to the display units.
There was a problem hiding this comment.
Ok this makes sense now. Maybe the AtomicToEVM and EVMToAtomic methods could be renamed, since they are converting between the atomic units in the contract and the atomic units in the dex. Haven't thought of a better name yet though.
There was a problem hiding this comment.
I tried a few different names and this was the best I could come up with. DEXToEVM isn't really descriptive, DEXAtomicToETHAtomic is nonsense. This seemed to be a reasonable middle ground. We use "atomic" to refer to the DEX units throughout, so even though it's a misnomer in this context, I thought we'd all understand.
| @@ -0,0 +1,75 @@ | |||
| // This code is available on the terms of the project LICENSE.md file, | |||
There was a problem hiding this comment.
Should this file just go into dex/networks/eth? Neither client or server have an erc20 folder, so I think we should just get rid of the dex/networks/erc20 folder.
There was a problem hiding this comment.
There's other stuff in the dex/networks/erc20 folder, but do I understand that you mean that the new txdata stuff and existing params stuff could be moved to dexeth? I personally feel like we should keep the packages separate, but I really hadn't though much about it. I don't see any reason why the networks folder needs to match the structure of either client or server. I guess there might be some other advantages to combining though. I wonder how this choice might affect the future implementation of other EVM assets. If you think it's important enough, you could open an issue.
| // EVMFactor allows for arbitrary ERC20 decimals. For an ERC20 contract, | ||
| // the relation | ||
| // math.Log10(UnitInfo.Conventional.ConversionFactor) + Token.EVMFactor = decimals | ||
| // should hold true. | ||
| // Since most assets will use a value of 9 here, a default value of 9 will | ||
| // be used in AtomicToEVM and EVMToAtomic if EVMFactor is not set. | ||
| EVMFactor *int64 `json:"evmFactor"` // default 9 |
There was a problem hiding this comment.
I've read the discussion you had with @martonp on this, but I'm still a bit fuzzy on this.
math.Log10(UnitInfo.Conventional.ConversionFactor) + Token.EVMFactor = decimals
Is 9 still an expected EVMFactor default? Sounded like you would have 6 and 0?
Take the testTokenID, defined below. It has ConversionFactor of 1e8 and nil for EVMFactor. The factor() method would return 10^9. But log10(conversionFactor) is 8, so that makes decimals get computed as ... 17?
There was a problem hiding this comment.
The ConversionFactor for testTokenID is GweiFactor = 1e9, so decimals is 18, which is the default for a token if not specified.
Some of the token primitives used in decred#1399, offered separately here to ease the integration of that work. dex: Add a simple Token type, outlining the parent-child link and the UnitInfo. Move other token details to dex/networks/eth dex/networks/erc20: Rename TOKEN_ADDRESS method to token_address so that the Go method will be TokenAddress instead of TOKENADDRESS. Add tx data parsers for transfer and transferFrom calls. dex/networks/eth: Export calldata parsing machinery. Add token details and Gases that used to be in /dex. Add methods for handling tokens with decimals != 18.
4913a3d to
e2f7a43
Compare
Some of the token primitives used in #1399, offered separately here to ease the integration of that work.