ckb-cell / rgbpp-sdk

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

fix(btc): only add paymaster output when needed in sendRgbppUtxos() API #135

Closed ShookLyngs closed 4 months ago

ShookLyngs commented 4 months ago

Changes

Details

Currently, the logic of needPaymasterOutput is simple:

const needPaymasterOutput = CKB_VTX.inputs.length < CKB_VTX.outputs.length;

As you can see, we're comparing the length of inputs and outputs to see if the CKB virtual transaction needs a paymaster cell. It's a solid solution and can work without issues in most cases. However, in some rare cases, the information can be inaccurate.

For example, if the following conditions are met:

  1. Transferring a single spore (the CKB_VTX has 1 input and 1 output)
  2. The target spore itself should pay for the transaction fee, but it has ran out of capacity margin
  3. Although the length of inputs and outputs are both 1, the transaction still needs extra capacity to pay for the fee

Right now we don't have to worry about those rare cases, but in the future if needed, we can improve the check statement by comparing the capacity sum of the inputs and outputs of the CKB_VTX, instead of comparing their lengths. For that, we need to query each input's corresponding Live Cell (because inputs are OutPoints that don't contain the capacity property):

let inputsCapacity: number = 0;
for (let i = 0; i < CKB_VTX.inputs.length; i++) {
  const input: OutPoint = CKB_VTX.inputs[i];
  const liveCell: LiveCell = await rpc.getLiveCell(input);
  inputsCapacity += BI.from(liveCell.output.capacity).toNumber();
}
ahonn commented 4 months ago

The test seems to need fixing, consider switching to our paid mempool.space api

ShookLyngs commented 4 months ago

The test seems to need fixing, consider switching to our paid mempool.space api

Yes, I was waiting for the update of btc-assets-api: https://github.com/ckb-cell/btc-assets-api/pull/96. Do you think I should quit waiting and just switch to the paid api ASAP? My concern was that it wouldn't help much, as I've tested with the paid api but the 503 error still occurs from time to time.

Dawn-githup commented 4 months ago

The test seems to need fixing, consider switching to our paid mempool.space api

Yes, I was waiting for the update of btc-assets-api: ckb-cell/btc-assets-api#96. Do you think I should switch to the paid api instead of waiting? I've tested with the paid api and it help just a little, the 503 error still occurs from time to time.

When encountering a 503 error, can it be avoided by retrying a few times?

ShookLyngs commented 4 months ago

When encountering a 503 error, can it be avoided by retrying a few times?

Yes, adding retry mechanism to the BtcAssetsApi is also a solution.