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

feat: support spore nft #2810

Merged
merged 27 commits into from Nov 15, 2023
Merged

feat: support spore nft #2810

merged 27 commits into from Nov 15, 2023

Conversation

homura
Copy link
Collaborator

@homura homura commented Aug 21, 2023

Resolves: Magickbase/neuron-public-issues#252

image

Limitation with The Light Client

  • expected: [ID][Cluster Name] Spore
  • actual: [ID][Cluster ID] Spore

When working with the light client in this PR, the [Cluster Name] part is displayed as a Cluster ID. This is because it is difficult to resolve the cluster cell from a spore cell, which is not owned by the wallet. To search the cluster cell that derives the spore cell, we need to:

  1. Decode the cluster_id from a spore cell's data if the spore is born from a cluster
  2. Build the type script filter of code_hash: cluster_code_hash, data: data1, args: cluster_id and search in the CKB indexer
  3. Repeat step 2 with another version of cluster scripts if the cluster cell is not found.

This limitation appears because Neuron only subscribes to lock scripts owned by a wallet, while the cluster cell may not belong to it.

Although we can subscribe to the cluster cells not owned by the wallet, it will make the light client heavier. If we subscribe to more scripts to make the features the same as the full node, the light client will lose its lightness. Therefore, I believe that the trade-off here may be acceptable.

Notes

  • Cluster cells can NOT be destroyed

@homura homura marked this pull request as ready for review August 30, 2023 09:12
@homura
Copy link
Collaborator Author

homura commented Aug 30, 2023

/package
Packaging for test is done in 6022935795. @homura

@homura
Copy link
Collaborator Author

homura commented Aug 30, 2023

I found there were some circular dependencies in Neuron, maybe we should find a way to avoid it

Details

 neuron-wallet git:(spore) ✗ dpdm --circular --tree false src/**/*.ts     
✔ [221/221] Analyze done!
• Circular Dependencies
  01) src/models/keys/key.ts -> src/models/keys/address.ts
  02) src/models/chain/transaction.ts -> src/models/offline-sign.ts
  03) src/database/chain/entities/transaction.ts -> src/database/chain/entities/input.ts
  04) src/database/chain/entities/transaction.ts -> src/database/chain/entities/output.ts
  05) src/database/chain/entities/asset-account.ts -> src/database/chain/entities/sudt-token-info.ts
  06) src/services/tx/transaction-service.ts -> src/utils/export-history.ts
  07) src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-service.ts -> src/utils/export-history.ts
  08) src/controllers/app/menu.ts -> src/controllers/export-debug.ts -> src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/controllers/sync-api.ts -> src/services/node.ts -> src/services/ckb-runner.ts -> src/services/settings.ts
  09) src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/controllers/sync-api.ts -> src/services/node.ts -> src/services/ckb-runner.ts -> src/services/indexer.ts -> src/database/chain/index.ts -> src/services/sync-progress.ts -> src/services/wallets.ts
  10) src/block-sync-renderer/index.ts -> src/controllers/sync-api.ts -> src/services/node.ts -> src/services/ckb-runner.ts -> src/services/indexer.ts -> src/database/chain/index.ts -> src/services/sync-progress.ts -> src/services/wallets.ts
  11) src/services/ckb-runner.ts -> src/services/indexer.ts -> src/services/monitor/index.ts -> src/services/monitor/ckb-monitor.ts
  12) src/block-sync-renderer/index.ts -> src/controllers/sync-api.ts -> src/services/node.ts -> src/services/ckb-runner.ts -> src/services/indexer.ts -> src/services/monitor/index.ts -> src/services/monitor/ckb-monitor.ts -> src/services/light-runner.ts
  13) src/services/node.ts -> src/services/ckb-runner.ts -> src/services/indexer.ts -> src/services/monitor/index.ts -> src/services/monitor/ckb-monitor.ts
  14) src/services/node.ts -> src/services/ckb-runner.ts -> src/services/indexer.ts
  15) src/block-sync-renderer/index.ts -> src/controllers/sync-api.ts -> src/services/node.ts -> src/services/ckb-runner.ts -> src/services/indexer.ts
  16) src/block-sync-renderer/index.ts -> src/controllers/sync-api.ts -> src/services/node.ts -> src/services/ckb-runner.ts
  17) src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts
  18) src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/tx-status-listener.ts
  19) src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts
  20) src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts -> src/services/asset-account-service.ts
  21) src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts -> src/services/asset-account-service.ts -> src/services/transaction-sender.ts
  22) src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts -> src/services/asset-account-service.ts -> src/services/transaction-sender.ts
  23) src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts -> src/services/asset-account-service.ts -> src/services/transaction-sender.ts -> src/services/hardware/index.ts -> src/services/hardware/ledger.ts -> src/services/hardware/hardware.ts
  24) src/services/transaction-sender.ts -> src/services/hardware/index.ts -> src/services/hardware/ledger.ts -> src/services/hardware/hardware.ts
  25) src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts -> src/services/asset-account-service.ts -> src/services/transaction-sender.ts
  26) src/services/tx/index.ts -> src/services/tx/transaction-generator.ts -> src/services/cells.ts -> src/services/live-cell-service.ts -> src/block-sync-renderer/index.ts -> src/block-sync-renderer/task.ts -> src/block-sync-renderer/sync/queue.ts -> src/services/asset-account-service.ts
  27) src/services/addresses.ts -> src/services/tx/index.ts -> src/services/tx/transaction-generator.ts
  28) src/controllers/app/index.ts -> src/controllers/api.ts -> src/controllers/app/show-window.ts

packages/neuron-wallet/package.json Outdated Show resolved Hide resolved
packages/neuron-ui/src/utils/formatters.ts Outdated Show resolved Hide resolved
packages/neuron-wallet/src/services/cells.ts Outdated Show resolved Hide resolved
@homura
Copy link
Collaborator Author

homura commented Sep 7, 2023

/package
Packaging for test is done in 6109098199. @homura

# Conflicts:
#	packages/neuron-wallet/package.json
#	yarn.lock
@homura
Copy link
Collaborator Author

homura commented Oct 27, 2023

@silySuper

1.In full node mode,NFT can be sent to itself(input address equals output address).In expect ,it is an useful operation.

I'm a bit confused about this point. Does it say that we should prevent users from transferring spores to the same address?

2.In full node mode,when NFT is sent to address it shows successfully,but in address book it balance does not change.In expect the output address balance will change.

I think is a feature instead of a bug because Nueron only treats the cells without data as spendable cells, so I suggest we keep the same here. @Keith-CY @yanguoyu What are your ideas about this point?

output.hasData = false AND
output.typeHash is null

@silySuper
Copy link
Collaborator

@silySuper

1.In full node mode,NFT can be sent to itself(input address equals output address).In expect ,it is an useful operation.

I'm a bit confused about this point. Does it say that we should prevent users from transferring spores to the same address?

2.In full node mode,when NFT is sent to address it shows successfully,but in address book it balance does not change.In expect the output address balance will change.

I think is a feature instead of a bug because Nueron only treats the cells without data as spendable cells, so I suggest we keep the same here. @Keith-CY @yanguoyu What are your ideas about this point?

output.hasData = false AND
output.typeHash is null

For No.1 send NFT to itself is a useful operation? @Danie0918 if it is not ,it is better to have a tip for users.

@yanguoyu
Copy link
Collaborator

@silySuper

1.In full node mode,NFT can be sent to itself(input address equals output address).In expect ,it is an useful operation.

I'm a bit confused about this point. Does it say that we should prevent users from transferring spores to the same address?

2.In full node mode,when NFT is sent to address it shows successfully,but in address book it balance does not change.In expect the output address balance will change.

I think is a feature instead of a bug because Nueron only treats the cells without data as spendable cells, so I suggest we keep the same here. @Keith-CY @yanguoyu What are your ideas about this point?

output.hasData = false AND
output.typeHash is null

I'm sure that balance does not include cells' capacity that the cells include data. Neurons will not cost the cells that have data when transferring CKB.

@Danie0918
Copy link
Collaborator

@silySuper

1.In full node mode,NFT can be sent to itself(input address equals output address).In expect ,it is an useful operation.

I'm a bit confused about this point. Does it say that we should prevent users from transferring spores to the same address?

2.In full node mode,when NFT is sent to address it shows successfully,but in address book it balance does not change.In expect the output address balance will change.

I think is a feature instead of a bug because Nueron only treats the cells without data as spendable cells, so I suggest we keep the same here. @Keith-CY @yanguoyu What are your ideas about this point?

output.hasData = false AND
output.typeHash is null

For No.1 send NFT to itself is a useful operation? @Danie0918 if it is not ,it is better to have a tip for users.

This is something that can be initiated normally without prompting, similar to sending CKB.

@homura
Copy link
Collaborator Author

homura commented Nov 6, 2023

Screen.Recording.2023-11-06.at.16.44.16.mov

Transferring spores via the light client is supported, please have a check, @silySuper

@silySuper
Copy link
Collaborator

silySuper commented Nov 7, 2023

/package
Packaging for test is done in 6778653910. @silySuper

@silySuper
Copy link
Collaborator

@silySuper

1.In full node mode,NFT can be sent to itself(input address equals output address).In expect ,it is an useful operation.

I'm a bit confused about this point. Does it say that we should prevent users from transferring spores to the same address?

2.In full node mode,when NFT is sent to address it shows successfully,but in address book it balance does not change.In expect the output address balance will change.

I think is a feature instead of a bug because Nueron only treats the cells without data as spendable cells, so I suggest we keep the same here. @Keith-CY @yanguoyu What are your ideas about this point?

output.hasData = false AND
output.typeHash is null

For No.1 send NFT to itself is a useful operation? @Danie0918 if it is not ,it is better to have a tip for users.

This is something that can be initiated normally without prompting, similar to sending CKB.

For No.1,send CKB will have tip when send and receive is same.So send NFT need tip @Danie0918 ?

截屏2023-11-08 17 07 32

@Danie0918
Copy link
Collaborator

@silySuper

1.In full node mode,NFT can be sent to itself(input address equals output address).In expect ,it is an useful operation.

I'm a bit confused about this point. Does it say that we should prevent users from transferring spores to the same address?

2.In full node mode,when NFT is sent to address it shows successfully,but in address book it balance does not change.In expect the output address balance will change.

I think is a feature instead of a bug because Nueron only treats the cells without data as spendable cells, so I suggest we keep the same here. @Keith-CY @yanguoyu What are your ideas about this point?

output.hasData = false AND
output.typeHash is null

For No.1 send NFT to itself is a useful operation? @Danie0918 if it is not ,it is better to have a tip for users.

This is something that can be initiated normally without prompting, similar to sending CKB.

For No.1,send CKB will have tip when send and receive is same.So send NFT need tip @Danie0918 ?

截屏2023-11-08 17 07 32

If you can receive the NFT normally you don't need to be prompted, it's a limitation of the APC account here, isn't it?

@silySuper
Copy link
Collaborator

No.3 Verified.

@silySuper
Copy link
Collaborator

No.1 and No.2 not need to change.

@silySuper
Copy link
Collaborator

silySuper commented Nov 9, 2023

  1. Offline sign can not send successfully when transaction is signed and export in light client,appear uncertainly
2023-11-09.17.13.06.mov

@silySuper
Copy link
Collaborator

Light client sync failed. @homura
Testing package is this:
https://github.com/nervosnetwork/neuron/actions/runs/6778653910

This is the debug log.
neuron_debug_1699860419192.zip

@homura
Copy link
Collaborator Author

homura commented Nov 13, 2023

This is the debug log.

Hi @silySuper, did you run more than one light client at the same during the test? If yes, then the light client may not be working correctly.

Failed to open rocksdb: Error { message: "IO error: While lock file:/... Resource temporarily unavailable" }.
...
function call error: Error: connect ECONNREFUSED 127.0.0.1:9001, retry 2 

@silySuper
Copy link
Collaborator

This is the debug log.

Hi @silySuper, did you run more than one light client at the same during the test? If yes, then the light client may not be working correctly.

Failed to open rocksdb: Error { message: "IO error: While lock file:/... Resource temporarily unavailable" }.
...
function call error: Error: connect ECONNREFUSED 127.0.0.1:9001, retry 2 

Today from activity monitor,there is no ckb process and can sync.

@silySuper
Copy link
Collaborator

Send feature has verified.

@homura
Copy link
Collaborator Author

homura commented Nov 15, 2023

Today from activity monitor,there is no ckb process and can sync.

So you mean that the sync of the light client is working well, right?

Send feature has verified.

Does it mean that the PR is completely tested?

@silySuper
Copy link
Collaborator

Today from activity monitor,there is no ckb process and can sync.

So you mean that the sync of the light client is working well, right?

Send feature has verified.

Does it mean that the PR is completely tested?

Yes

@Keith-CY Keith-CY added this pull request to the merge queue Nov 15, 2023
Merged via the queue into nervosnetwork:develop with commit 8abdab0 Nov 15, 2023
10 checks passed
@homura homura deleted the spore branch November 15, 2023 10:42
@Keith-CY Keith-CY mentioned this pull request Nov 23, 2023
@Keith-CY Keith-CY mentioned this pull request Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting Spore
6 participants