Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize input and output storage #77

Closed
yanguoyu opened this issue Nov 23, 2022 · 14 comments
Closed

Optimize input and output storage #77

yanguoyu opened this issue Nov 23, 2022 · 14 comments
Assignees
Labels
enhancement New feature or request

Comments

@yanguoyu
Copy link

Now, every input cell and output cell in the transaction that includes the user's cell will save in SQLite.
If a user deposits ckb from an exchange, the transaction will include many inputs and outputs. I found that a user deposits many times from an exchange. And then the input entity and output entity have 500,000 rows and 1000,000 rows, but most of them are not belong to the user. It may affect SQL execution time.
So we can save input cells and output cells that only belong to the users, and if we need to get the transaction's detail, we can get it by RPC service.

@yanguoyu yanguoyu added the enhancement New feature or request label Nov 23, 2022
@yanguoyu yanguoyu self-assigned this Nov 23, 2022
@Danie0918 Danie0918 added this to Neuron Feb 26, 2023
@Danie0918 Danie0918 moved this to 📋 Backlog in Neuron Feb 26, 2023
@Danie0918 Danie0918 moved this from 📋 Backlog to 📌Planning in Neuron Mar 22, 2023
@yanguoyu
Copy link
Author

This scheme does not work, because in the history page, users can search by other's address.
https://github.com/nervosnetwork/neuron/blob/v0.106.0/packages/neuron-wallet/src/services/tx/transaction-service.ts#L74-L98

@yanguoyu
Copy link
Author

How about searching by https://github.com/nervosnetwork/ckb-indexer#get_transactions when the user search's address is not theirs? @Keith-CY
And I need to check whether there are exist any else that needs other's cells.

@Keith-CY
Copy link
Member

How about searching by nervosnetwork/ckb-indexer#get_transactions when the user search's address is not theirs? @Keith-CY And I need to check whether there are exist any else that needs other's cells.

It seems work

@yanguoyu
Copy link
Author

How about searching by nervosnetwork/ckb-indexer#get_transactions when the user search's address is not theirs? @Keith-CY And I need to check whether there are exist any else that needs other's cells.

It seems work

There may exist a problem if the user enters an address(e.g. a miner's address) that has many txs, the RPC may use more times to search txs.
It's better to get the current wallet's all tx details, then we should use batch request get_transaction RPC to get all transactions' detail, and then filter by lockArgs.

@yanguoyu
Copy link
Author

yanguoyu commented Mar 30, 2023

It will implement by these steps:

  1. Write migrate to clear InputEntity, OutputEntity, TransactionEntity, IndexerTxHashCache
  2. When calling https://github.com/nervosnetwork/neuron/blob/v0.106.0/packages/neuron-wallet/src/block-sync-renderer/sync/queue.ts#L103, remove the inputs and outputs that not belong to the current wallet.
  3. When searching transactions by address, if this address belongs to the current wallet, keep the old logic, else get all transactions' detail in the current wallet and then filter with the address.
  4. When getting transaction detail, use call get_transaction to replace.

@Keith-CY
Copy link
Member

Keith-CY commented Mar 30, 2023

  1. When calling https://github.com/nervosnetwork/neuron/blob/v0.106.0/packages/neuron-wallet/src/block-sync-renderer/sync/queue.ts#L103, remove the inputs and outputs that not belong to the current wallet.

This method will be called multiple times and most of them are useless, how about remove the historical data by clear cache

@yanguoyu
Copy link
Author

This method will be called multiple times and most of them are useless, how about remove the historical data by clear cache

I don't get this point. This method is used to sync transactions to DB. I mean remove input and ouputs that don't belong to the current wallet before saving them to DB.

@Keith-CY
Copy link
Member

This method will be called multiple times and most of them are useless, how about remove the historical data by clear cache

I don't get this point. This method is used to sync transactions to DB. I mean remove input and ouputs that don't belong to the current wallet before saving them to DB.

I see, I thought it's used to remove historical data

@yanguoyu
Copy link
Author

This method will be called multiple times and most of them are useless, how about remove the historical data by clear cache

I don't get this point. This method is used to sync transactions to DB. I mean remove input and ouputs that don't belong to the current wallet before saving them to DB.

I see, I thought it's used to remove historical data

I will write migrate to remove historical data in step 1, it's the same as clear cache.

@yanguoyu
Copy link
Author

yanguoyu commented Mar 30, 2023

I found another problem, when we get the transaction list we need to get all input and output to calculate receive ckb or send ckb.
It seems that there is no need to optimize because getting the transaction list is a frequent operation.
@Keith-CY What do you think?

@Keith-CY
Copy link
Member

I found another problem, when we get the transaction list we need to get all input and output to calculate receive ckb or send ckb. It seems that there is no need to optimize because getting the transaction list is a frequent operation. @Keith-CY What do you think?

But the input/output owned by other addresses are not used in the computation, they are still removable

@yanguoyu
Copy link
Author

yanguoyu commented Mar 30, 2023

But the input/output owned by other addresses are not used in the computation, they are still removable

Sound like that, I make a mistake to think it needs another's cell to calculate.

@Danie0918 Danie0918 moved this from 📌Planning to 🏗 In Progress in Neuron Apr 10, 2023
@Danie0918 Danie0918 assigned Danie0918 and unassigned yanguoyu Apr 10, 2023
@Danie0918 Danie0918 moved this from 🏗 In Progress to 📌Planning in Neuron Apr 15, 2023
@Danie0918 Danie0918 moved this from 📌Planning to 🏗 In Progress in Neuron Apr 20, 2023
@Danie0918 Danie0918 assigned yanguoyu and unassigned Danie0918 Apr 20, 2023
@yanguoyu
Copy link
Author

Here https://github.com/nervosnetwork/neuron/blob/develop/packages/neuron-wallet/src/block-sync-renderer/sync/indexer-cache-service.ts#L81 also can be optimized, it gets all transactions from the node every time. We can cache the last block number and get it from the last block number next time.

@yanguoyu
Copy link
Author

@Danie0918 Danie0918 moved this from 🏗 In Progress to 👀 Testing in Neuron May 29, 2023
@Danie0918 Danie0918 moved this from 👀 Testing to 🚩Pre Release in Neuron Jun 26, 2023
@Danie0918 Danie0918 moved this from 🚩Pre Release to ✅ Done in Neuron Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Status: Todo
Development

No branches or pull requests

7 participants