ckb-cell / rgbpp-sdk

Utilities for Bitcoin and RGB++ asset integration
ISC License
53 stars 16 forks source link

feat(rgbpp): support for batch transferring of RGBPP XUDT assets #270

Closed ShookLyngs closed 1 month ago

ShookLyngs commented 1 month ago

Changes

TODOs

ShookLyngs commented 1 month ago

Mocked a test for the transfer-all-of-an-rgbpp-xudt-asset feature with @Dawn-githup:

The expected result:

The actual transaction groups generated from the process:

Flouse commented 1 month ago
  • The L1 fees should be paid with the L1 RGBPP outputs because the fee rate is low

Have you tested the case where the BTC transaction requires additional inputs to pay tx fee? @ShookLyngs @Dawn-githup

Dawn-githup commented 1 month ago
  • The L1 fees should be paid with the L1 RGBPP outputs because the fee rate is low

Have you tested the case where the BTC transaction requires additional inputs to pay tx fee? @ShookLyngs @Dawn-githup

When the fee rate is higher, utxo will be collected from the from address to pay the fee.

Flouse commented 1 month ago

Discussion or TODO

ShookLyngs commented 1 month ago

@duanyytop and I had a few discussions about the PR:

  1. The .env file for the rgbpp lib is redundant, as all tests in the rgbpp lib are mocked. Can it be removed?
  2. Should we keep the rgbpp lib as simple as possible, instead of adding a lot of utilities to it?
  3. Developers may need an example for the transfer-all feature when coding.

My thoughts:

The .env file for the rgbpp lib is redundant, as all tests in the rgbpp lib are mocked. Can it be removed?

The btc/ckb lib contain some methods that are not mockable, meaning that when calling the buildRgbppTransferAllTxs API, actual network requests must be sent. So, while the .env file can be removed, we still need to provide a default environment so the tests can run as expected. I'm not sure if it's worth it.

I think @duanyytop is concerned about redundant test cases, as there are too many "same pieces of code" in the examples and integration tests. If we add more tests to the rgbpp lib alone, that would increase redundancy.

Personally, I believe the unit tests for the rgbpp library are different from the examples and integration tests because the unit tests allow more room for unexpected cases. However, writing tests for the buildRgbppTransferAllTxs API is challenging and requires a lot of preparation. Maybe @Dawn-github can write integration tests for the feature, and then we can remove the mocked tests from the rgbpp lib, making the .env file unnecessary.

Should we keep the rgbpp lib as simple as possible, instead of adding a lot of utilities to it?

Agreed, so I moved most of the utilities from the rgbpp lib to the btc/ckb libs at c492cf3.

Developers may need an example for the transfer-all feature when writing.

Yes, but it would be hard to write a set of examples just to prepare the assets. If we provide only one example for users who already have many assets, it would be much easier. We may add such example in examples/rgbpp/xudt/btc-transfer-all/1-btc-transfer-all.ts.

Dawn-githup commented 1 month ago

Regarding the transfer-all example, we are currently facing an issue: if we use this file to create a BTC account and sign transactions, with createBtcAccount it won’t support mixed-type addresses when using .env.

@Flouse @ShookLyngs

ShookLyngs commented 1 month ago

Regarding the transfer-all example, we are currently facing an issue: if we use this file to create a BTC account and sign transactions, with createBtcAccount it won’t support mixed-type addresses when using .env.

I think it depends on how complicated the examples need to be. If we're writing a simple example that transfers all XUDT1 from A to B, then a single account is enough. However, if we're writing an example that transfers all XUDT1 from [A, B] to C, then we'll need at least 2 accounts for signing.

It's best to draft a TODO list of the examples first. cc @Flouse @Dawn-githup

Dawn-githup commented 4 weeks ago

It's best to draft a TODO list of the examples first. cc @Flouse @Dawn-githup

https://github.com/ckb-cell/rgbpp-sdk/commit/10f045774d705adbf44b1dcf3bf3ce34866a433b

Flouse commented 4 weeks ago
  • This is a simple example using a single account transfer-all

10f0457

ok, please add an introduction to https://github.com/ckb-cell/rgbpp-sdk/blob/develop/examples/rgbpp/README.md to explain the use case of this example later.