-
Notifications
You must be signed in to change notification settings - Fork 3
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
IEthereumTransaction #1
Comments
Decimal has 28 to 29 significant digits, which will be sufficient to hold any valid amount if Ethereum caps at 120m as suggested in this Ethereum EIP issue (ethereum/EIPs#960). However, I prefer using BigInteger to represent the WEI (or Satoshi in BTC) plus another field indicating the decimal places (18 for Ethereum, 8 for Bitcoin) of the most common used unit (e.g. ETH, BTC). It is more future proof using BigInteger and also reduces confusions when a coin has multiple denominations. |
@touchjet thanks for the input. Yes, this seems like the consensus. I'll be putting this together soon. Sorry it's taking a while . |
How about this (https://github.com/MelbourneDeveloper/Hardwarewallets.Net/blob/IEthereumWallet/src/Hardwarewallets.Net/Ethereum/IEthereumTransaction.cs):
Touch jet, I've made GasPrice BigInteger because this is in GWEI? But, because GasLimit is only units of GWEI, it doesn't need to be a BigInteger. Does this sound right to you both? |
I believe that GasPrice will always be within the decimal range, while Value could possibly (although the chance is next to zero) be out of range. |
@MelbourneDeveloper Did you mean the GasPrice would be in WEI? Because if you intend on the gas price being in GWEI, you would need to change the data type to decimal most likely since some transactions go through at decimal GWEI values (e.g. 2.3). If you want to go this route, then it would make more sense to make the GasPrice of decimal data type so that it would function properly in GWEI. As @touchjet talked about, there probably aren't any issues with the actual range of the data types. Either way, I think you should choose some consistency between the GasPrice and Value. You could either make it so that they are both of the type decimal, and the GasPrice would be in GWEI while the Value would be in ETHER. Or, you have the option of using BigInteger for both of them and having them simply represented in WEI. I see the latter as a more uniform and consistent solution (I am biased since I am used to it) but it is definitely less comprehensive when you are new to dealing with this stuff. |
@MelbourneDeveloper In Nethereum everything is a BigInteger or HexBigInteger for RPC, then let the user do conversions as we don't know the future (even if stable). I also provide now a generic service that accepts decimals to Transfer Ether and does the conversion internally (Ether to Wei, and Gwei to Wei) |
One field at a time... GasPrice: @ThatSlyGuy , I see. Would you use BigInteger WEI, or decimal GWEI? I'd just like to follow the Ethereum standard on this one. GasLimit: What should this be? Value: @juanfranblanco , The Trezor accepts decimal and this is compatible with other coins, so I'm tempted to go in that direction. But if you think it is not future proof, I can change it. Is it possible or likely that in future values will become larger than a System.Decimal can represent? I like the idea of uniformity as well. The problem is that if I keep uniformity with WEI - internal to Ethereum, I lose uniformity with other coins. It's an especially big problem in apps that are doing transactions. I don't want to have scenario where someone writes an app, and puts through a transaction for say 1 ETH thinking that this is a reasonable value because all the other coins accept that value but they are actually attempting to send 1 WEI. |
Yeah I think if anything you should keep things consistent with other coins. If you intend on having the Bitcoin transaction interface using the transaction value in Bitcoin instead of Satoshi, you should do the same for Ethereum. One thing to keep in mind though, if you do it this way, you'll need to have some sort of custom conversion behind the scenes before you send the values to the Ledger/Trezor. For example, if the Bitcoin transaction interface took in the send value in Bitcoin, you would probably need to convert it to the Satoshi value before actually sending the transaction to the device. Same goes for Ethereum, and every other coin most likely. With regards to the GasPrice though, honestly I would prefer it in WEI. Every library I've seen has used their gas price and gas limit values in WEI, so I am just used to that. I think the question comes down to which people will use your library. Will it be seasoned developers who are already aware of the standard ETHER/GWEI/WEI conversions, or will it be mainly newer people to the scene who want to be able to quickly implement some hardware wallet support for their app. My guess is it would be more seasoned developers who are used to this conversion. It's really something everybody who gets into the scene has to tackle and understand at some point. Apologies if this comment is all over the place... it is late over here and I am quite tired. Hope some of these thoughts help a bit though. |
@MelbourneDeveloper @ThatSlyGuy I agree with @ThatSlyGuy, mainly a experienced developer will interact with the hardware wallets and should know about the conversion so at lower level it will be necessary.. Decimals that is an interesting point, I created BigDecimal for Nethereum (that explains it all :) ) Although Ether in particular might not be an issue for many years. |
The IEthereumTransaction interface is being constructed. This interface represents an Ethereum transaction to be signed by a hardware wallet. I'm looking for people to review this structure as it will be rolled out across Ledger.Net, Trezor.Net, and KeepKey.Net.
The code is here:
https://github.com/MelbourneDeveloper/Hardwarewallets.Net/blob/IEthereumWallet/src/Hardwarewallets.Net/Ethereum/IEthereumTransaction.cs
One questionable aspect of this interface is that I have suggested that decimal data types should be used. In each case, the decimal represents the value in ETH - not WEI or GWEI. The rationale behind this is that this library sits on top of multiple different coin types, and to maintain compatibility across coins, a standard should be used to represent values as users would see them on exchanges and so on. WEI and GWEI are Ethereum specific just as Satoshis are Bitcoin specific.
However, i can't guarantee that decimals will be able to represent all ETH values. Can I get a confirmation on whether or not decimal is reasonable? Should we use BigInteger? If so, can we at least keep a standard of one unit of measure instead of three different ones (WEI, GWEI, ETH)?
@thatslyguy, @juanfranblanco
The text was updated successfully, but these errors were encountered: