Thoughts on API changes
mahnunchik opened this issue · 3 comments
I've faced with the several strange API changes when upgrading micro-eth-signer
from version 0.6.4 to 0.9.1 CoinSpace/cs-evm-wallet@7f0e6c7
Thoughts on API changes:
- It is much easier to use and maintain if the library uses values in a single scale, such as cryptocurrency atoms (wei for Ethereum). In version 0.9.1 the library returns wei and humanized formats in
calcAmounts()
method. It complicates the library internally, but in the application it is most likely required to format number again (for example if trailing zeros needed). - The reason for adding
calcAmounts()
method is unclear. It seems much clearer for the end user to use getters such.fee
,.amount
. - Precalculation of
amountWithFee
may be easy calculated (fee + amount) and may not be required by the consumer at all.
The rest of the changes between version in the library seem quite logical.
yeah, humanized
is not ideal and can likely be dropped because >the application it is most likely required to format number again
It seems much clearer for the end user to use getters
I've dropped all getters except for hash
, sender
and v
, because it had too many getters and it was hard to reason about the transaction: which fields are auto-calculated; which fields are not. After the update:
- The transaction can now be easily inspected in
console.log
where it always has humanized representation, instead of JSON-stringified crap like before v
is rarely used, so doesn't mattersender
is also rarely used - it's just an alias forrecoverSender().address
- if you need a public key, then it doesn't work: do
recoverSender().publicKey
- so just a small convenience function
- if you need a public key, then it doesn't work: do
hash
is more common. It can't be made pre-computed, because it would consume too much resourcesgetHash()
is weird nametoHash()
is also not great but I guess we can use it
Precalculation of amountWithFee may be easy calculated (fee + amount)
It's maximum amount that the transaction would consume. All fee estimations are "miscalculated", because gas price can't be known in advance — unless you set priorityFee = fee.
may not be required by the consumer at all
You're sending 0.5 eth with 20 gwei fee. Then you're actually spending 0.500042 eth (MAX) and not 0.5. That seems like a useful thing to know. Wallet can also present a UI where a user would enter they're wanting to spend exactly 0.5 eth. Then the wallet would re-calc the amount/fee.
As multi currency wallet app developer I can say that almost all open source wallet applications has almost the same architectecture:
- Underlying cryptocurrency library
Wallet
abstraction- UI application
For example edge-currency-accountbased and cs-evm-wallet
It is better if underlying library keep the balance between full featured
and simplicity
. It is easy to use, easy to audit and of course much easy to find a bugs.
As I already mentioned, we don't know exactly the use case, for example
- The application can show 5 transactions and that's it. Then there is no sense for caching and precomputation.
- In opposition, the application can juggle thousands of transactions. Then, in my opinion, caching and precomputation can be implemented in the
wallet abstraction
, not it underlying library to keep it simple. - Displaying the amount is the responsibility of the UI application.
amount
,fee
, andamountWithFee
can be shown in wei and humanized form. Or evengasLimit
can be shown. The need for a specific UI application is unknown for us.
My opinion is that it was more convenient to have a separate getter .fee
(or getter function .getFee()
) for each value without caching and precomputation.
For pretty console.log
inspection toString()
method can be implemented: https://github.com/CoinSpace/cs-common/blob/master/lib/Transaction.js#L58-L65
Of course it is better to have helpers
like ethDecimal
and gweiDecimal
.