search

Magickbase / neuron-public-issues

Neuron Issues
6 stars 3 forks source link

Refactoring With Lumos #177

Closed homura closed 6 months ago

homura commented 1 year ago

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

zhangyouxin commented 1 year ago

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 commented 1 year ago

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 commented 6 months ago

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 commented 6 months ago

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 ^-^