Cylix
commented
6 years ago I have copy/pasted the list, we can then build a checklist of what we should do as well and split that into independent issues.
Open Cylix opened 6 years ago
Cylix
commented
6 years ago I have copy/pasted the list, we can then build a checklist of what we should do as well and split that into independent issues.
Cylix
commented
6 years ago sendTransfer() was removed
prepareTransfers() + sendTrytes() should be used instead, with intermediate step of persisting transaction trytes. This extra step should be mentioned in docs for all methods that publish transactions (like in this case here)I am not sure to quite understand the point of removing this helper function, would need to check how iota.lib.py or iota.lib.java are heading regarding this.prepareTransfers() works offline: implementationWe need to double check, but I think we should already support offline. I had a quick overview, and seems the only time we need network in this method is if user does provided inputs and asked for an inputValidation. But maybe this validation process has been retired as well?getBundlesFromAddresses() is deprecated and will probably be removed in next major version.
Reason for deprecating is poor performance; number of bundle instances can be any, and fetching all bundles lazily is preferred for most use cases. I think that findTransactions() on addresses is fine for most cases.getTransfers() is deprecated
calls getBundlesFromAddressestransfers field of getAccountData() is deprecated
Reason for this is internal getBundlesFromAddresses call to fetch full bundles. transactions field was added as replacement which contains transactions (instead of bundles) as returned by findTransactions() on account addresses. (https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetAccountData.ts#L154)Everything is kind of related here and change sounds reasonable for the performance issues.
We already tried to have some improvements for getBundlesFromAddresses, but there is not so much room for improvement as most of the performance is bound by IRI execution time.isPromotable(tail) of develop branch is now renamed to checkConsistency(tail)isPromotable(tail, depth) is now expressed as function of checkConsistency(tail) & depth, because promotion becomes ineffective after a few milestones. Internally uses a time heuristic: https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createIsPromotable.tsSeems we have both isPromotable and checkConsistency, should we dismiss isPromotable?total, returnAll & checksum options of getNewAddress are deprecated
Reason is responsibility overload & goal is to have getNewAddress() do only one thing, what the name says. Next step is to have it work with local store and accept seed as the only argument.We already do not have checksum as it is moved to the Address model.
For returnAll, makes sense to remove it, this parameter does not bring much benefit and make API more complex.
For total, I am not sure about why removing it, probably leaving it optional should be fine as well?wereAddressesSpentFrom() is no longer exported - We don't want devs to build solutions around a method which might be infeasible in IoT environment. It's still being used under the hood in the following methods, until local store solution is implemented:
getNewAddress(): https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetNewAddress.ts#L23getAccountData(): https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetAccountData.ts#L168 & https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetAccountData.ts#L179getInputs(): need to be added - issue TBDTwo things here:
* For removing the export, we can move it as a private function and check with iota.lib.py and iota.lib.java about their position regarding this.
* We can look about how this function is used internally to use it as well if we don'tTheir code structure is pretty interesting with a repo split into independent modules.
We can probably do the same on our side in the future. We would then need to understand what each module is made for and what modules we can provide as well. We would also need to understand how to re-arrange our buildchain.validators package.Not sure what it is aboutWe can try to keep an eye an possibly be consistent in the futureWe can try to keep an eye an possibly be consistent in the futuregenerateAddress(seed, index, security = 2, checksum = false) for deterministic address generation: https://github.com/iotaledger/iota.lib.js/tree/next/packages/core#module_core.generateAddressWe already have a similar functionUnrelated to ustransactionTrytes/Object() to *as*TransactionTrytes/Object(). -asTransactionTrytes() accepts string or array of transaction trytes. Also added asTransactionObject*s* = hashes => trytes => objects which doesn't recalculate known hashes: https://github.com/iotaledger/iota.lib.js/tree/next/packages/transaction-converter#module_transaction-converter..asTransactionObjectsUnrelated to usNew padding utils: https://github.com/iotaledger/iota.lib.js/blob/next/packages/pad/src/index.ts
We already have some padding toolsEmphasis on simple tests.
Unrelated to usWe already handle thatWe already handle thatI created individual issues such that we can start working on each of them and discuss each one in a separate thread if necessary.
Changes in v1.0.0-alpha: (modifié)
sendTransfer()was removed -prepareTransfers()+sendTrytes()should be used instead, with intermediate step of persisting transaction trytes. This extra step should be mentioned in docs for all methods that publish transactions (like in this case here: https://github.com/iotaledger/iota.lib.js/tree/next/packages/core#module_core.storeAndBroadcast)prepareTransfers()works offline: https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/test/integration/prepareTransfers.test.ts#L46getBundlesFromAddresses()is deprecated and will probably be removed in next major version. - Reason for deprecating is poor performance; number of bundle instances can be any, and fetching all bundles lazily is preferred for most use cases. I think thatfindTransactions()on addresses is fine for most cases.getTransfers()is deprecated - callsgetBundlesFromAddressestransfersfield ofgetAccountData()is deprecated - Reason for this is internalgetBundlesFromAddressescall to fetch full bundles.transactionsfield was added as replacement which contains transactions (instead of bundles) as returned byfindTransactions()on account addresses. (https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetAccountData.ts#L154)isPromotable(tail)ofdevelopbranch is now renamed tocheckConsistency(tail)isPromotable(tail, depth)is now expressed as function ofcheckConsistency(tail)&depth, because promotion becomes ineffective after a few milestones. Internally uses a time heuristic: https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createIsPromotable.tsgenerateAddress(seed, index, security = 2, checksum = false)for deterministic address generation: https://github.com/iotaledger/iota.lib.js/tree/next/packages/core#module_core.generateAddresstotal,returnAll&checksumoptions ofgetNewAddressare deprecated - Reason is responsibility overload & goal is to havegetNewAddress()do only one thing, what the name says. Next step is to have it work with local store and accept seed as the only argument.wereAddressesSpentFrom()is no longer exported - We don't want devs to build solutions around a method which might be infeasible in IoT environment. It's still being used under the hood in the following methods, until local store solution is implemented:getNewAddress(): https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetNewAddress.ts#L23getAccountData(): https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetAccountData.ts#L168 & https://github.com/iotaledger/iota.lib.js/blob/next/packages/core/src/createGetAccountData.ts#L179getInputs(): need to be added - issue TBDtransactionTrytes/Object()to*as*TransactionTrytes/Object(). -asTransactionTrytes()accepts string or array of transaction trytes. Also addedasTransactionObject*s* = hashes => trytes => objectswhich doesn't recalculate known hashes: https://github.com/iotaledger/iota.lib.js/tree/next/packages/transaction-converter#module_transaction-converter..asTransactionObjectsChanges being worked on:
validatorspackage.