Refactoring With Lumos

[homura]

Background

Currently, there are several Lumos modules was reimplemented in Neuron, this results in higher maintenance cost for Neuron as the reimplementation causes us to rewrite unit tests

List

• neuron-wallet
  • LumosCell -> lumos/base/Cell - not compatible
  • LumosCellQuery -> lumos/ckb-indexer/QueryOptions - compatible
    • refactor: replace LumosCell, LumosCellQuery with Cell, QueryOptions nervosnetwork/neuron#3163
  • IndexerCacheService.fetchTxMapping -> TransactionCollector with grouped transaction feature
    • Move to backlog
• HexUtils -> @ckb-lumos/codec - compatible #187
• many HexString === HexString -> bytes.equal to avoid 0xAA !== 0xaa
  • Not found / Not important for now
• shannonToCKB, sudtValueToAmount-> BI.formatUnit
  • refactor: rewrite shannonToCKB with lumos formatUnit nervosnetwork/neuron#3114
  • refactor: rewrite sudtValueToAmount with formatUnit in neuron-wallet nervosnetwork/neuron#3119
  • refactor: rewrite sudtValueToAmount with formatUnit in neuron-ui nervosnetwork/neuron#3126
  • refactor: rewrite shannonToCKBFormatter with formatUnit nervosnetwork/neuron#3129
• models/blake2b -> lumos/base/utils/blake2b
• models/multisig -> lumos/base/since
  • refactor: refactor models/multisig with lumos nervosnetwork/neuron#3156
• refactor all the ckb indexer url with the ckb url, since the CKB Indexer has been integrated into CKB
  • refactor: remove indexerUrl nervosnetwork/neuron#3170
• models/keys -> @ckb-lumos/hd
  • refactor: replace models/keys with lumos hd nervosnetwork/neuron#3152
• @nervosnetwork/ckb-sdk-x can be refactored with the lumos

  • Refactor ckb-sdk-core by lumos #202

    • refactor: remove peerDependency of toolkit ckb-js/lumos#560
    • refactor: remove ckb-sdk-core and replace with lumos nervosnetwork/neuron#2843 (in test)

  • Refactor ckb-sdk-utils by lumos #201

    • refactor: remove address and script convertion from ckb-sdk-js nervosnetwork/neuron#2801
    • refactor: use lumos to calculate script hash nervosnetwork/neuron#2802
    • refactor: remove usage of serialization methods from ckb-sdk-js nervosnetwork/neuron#2803
    • refactor: remove systemScripts from ckb-sdk-js  nervosnetwork/neuron#2814
    • refactor: remove ckb-sdk-js from dependency list nervosnetwork/neuron#2817

  • the global variable ckbCore

    • refactor: replace toUint64Le, parseEpoch from ckbCore.utils with lumos nervosnetwork/neuron#3138
    • refactor: refactor ckbCore.utils.xyz with lumos methods nervosnetwork/neuron#3142
    • refactor: replace ckbCore with lumos CKBRPC nervosnetwork/neuron#3145

  • @nervosnetwork/ckb-sdk-utils

    • refactor: replace ckb-sdk-utils with lumos nervosnetwork/neuron#3149

[zhangyouxin]

We can seperate these tasks to small issues so that it is easy to write smaller PRs and can be parallelly executed by different team members.

[homura]

We can seperate these tasks to small issues so that it is easy to write smaller PRs and can be parallelly executed by different team members.

OK, I'll transform the plan section into TODO list

[homura]

IndexerCacheService.fetchTxMapping -> TransactionCollector with grouped transaction feature

The CKB indexer offers a feature group_by_transaction that allows the grouping of indexed transactions by their transaction hashes instead of having an array with multiple identical transactions. This feature is useful for scenarios where the io_index has not cared

We can simplify the fetchTxMapping as follows to reduce unnecessary lines and IO

const grouped = get_transactions({
  search_key: {
    lock: ...,
    group_by_transaction: true,
    limit: no_limit_here
  }
})

grouped.forEach(({tx_hash}) => {
   mappingsByTxHash.set(tx_hash, [...])
})

This is a small improvement, so I suggest we move the point to the backlog

[twhy]

IndexerCacheService.fetchTxMapping -> TransactionCollector with grouped transaction feature

The CKB indexer offers a feature group_by_transaction that allows the grouping of indexed transactions by their transaction hashes instead of having an array with multiple identical transactions. This feature is useful for scenarios where the io_index has not cared

We can simplify the fetchTxMapping as follows to reduce unnecessary lines and IO

const grouped = get_transactions({
  search_key: {
    lock: ...,
    group_by_transaction: true,
    limit: no_limit_here
  }
})

grouped.forEach(({tx_hash}) => {
   mappingsByTxHash.set(tx_hash, [...])
})

This is a small improvement, so I suggest we move the point to the backlog

Agree ^-^