iota.lib.js ongoing changes

[Cylix]

Hi, I've compiled a list of most significant changes in iota.js so far + short term roadmap. I suggest to discuss new specs and build a cross platform v1 API. Current v1-alpha API is still like fresh clay, we should form it for production readiness with precise decisions.
Other than that please ping here or DM me for anything you will need.

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#L46
• 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 getBundlesFromAddresses
• transfers 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)
• 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.ts
• Added generateAddress(seed, index, security = 2, checksum = false) for deterministic address generation: https://github.com/iotaledger/iota.lib.js/tree/next/packages/core#module_core.generateAddress
• 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.
• 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#L23
• getAccountData(): 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#L179
• getInputs(): need to be added - issue TBD
• Modularized from api, utils, crypto, multisig & valid to packages seen here: https://www.npmjs.com/org/iota - Everything is in iota.js monorepo, tested in integration and released with an even version across packages. Monorepos are really cool in our case! Please review, it's important to have high cohesion & low coupling! (would be nice to look for some good static code analysis tools too...)
• Fixed extended ascii issue in converter: https://github.com/iotaledger/iota.lib.js/issues/243
• Refactored bundle module: https://github.com/iotaledger/iota.lib.js/tree/next/packages/bundle#module_bundle
• Renamed transactionTrytes/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..asTransactionObjects
• New padding utils: https://github.com/iotaledger/iota.lib.js/blob/next/packages/pad/src/index.ts
• Emphasis on simple tests. (modifié)

Changes being worked on:

• Address validation strategy in relation to checksum: https://github.com/iotaledger/iota.lib.js/issues/245 - This requires refactoring of validators package.
• Request timeouts: https://github.com/iotaledger/iota.lib.js/pull/204
• Functional (end-to-end) tests: issue TBD
• Conventional commits & automatic releases: issue TBD
• Errors with standard codes & messages: issue TBD
• More enhancements soon:trade-mark:...

[Cylix]

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]

Changes we can work on / look deeper

sendTransfer

• [ ] 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.

prepareTransfer

• [ ] prepareTransfers() works offline: implementation

We 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

• [ ] 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 getBundlesFromAddresses
• [ ] transfers 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

• [ ] 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.ts

Seems we have both isPromotable and checkConsistency, should we dismiss isPromotable?

getNewAddress

• 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

• [ ] 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#L23
  • [ ] getAccountData(): 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#L179
  • [ ] getInputs(): need to be added - issue TBD

Two 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't

Bugfixes

• [ ] Fixed extended ascii issue in converter: https://github.com/iotaledger/iota.lib.js/issues/243

Long term changes

• [ ] Modularized from api, utils, crypto, multisig & valid to packages seen here: https://www.npmjs.com/org/iota - Everything is in iota.js monorepo, tested in integration and released with an even version across packages. Monorepos are really cool in our case! Please review, it's important to have high cohesion & low coupling! (would be nice to look for some good static code analysis tools too...)

Their 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.

Things to keep an eye on

• Address validation strategy in relation to checksum: https://github.com/iotaledger/iota.lib.js/issues/245 - This requires refactoring of validators package.

Not sure what it is about

• Conventional commits & automatic releases: issue TBD

We can try to keep an eye an possibly be consistent in the future

• Errors with standard codes & messages: issue TBD

We can try to keep an eye an possibly be consistent in the future

Changes we can skip

• Added generateAddress(seed, index, security = 2, checksum = false) for deterministic address generation: https://github.com/iotaledger/iota.lib.js/tree/next/packages/core#module_core.generateAddress

We already have a similar function

• Refactored bundle module: https://github.com/iotaledger/iota.lib.js/tree/next/packages/bundle#module_bundle

Unrelated to us

• [Renamed transactionTrytes/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..asTransactionObjects

Unrelated to us

• New padding utils: https://github.com/iotaledger/iota.lib.js/blob/next/packages/pad/src/index.ts

We already have some padding tools

• Emphasis on simple tests.

Unrelated to us

• Request timeouts: https://github.com/iotaledger/iota.lib.js/pull/204

We already handle that

• Functional (end-to-end) tests: issue TBD

We already handle that

[Cylix]

I created individual issues such that we can start working on each of them and discuss each one in a separate thread if necessary.