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(rgbpp-sdk/ckb): Add spv client cell and btc tx proof #22

Merged
merged 14 commits into from
Mar 15, 2024

Conversation

duanyytop
Copy link
Collaborator

Main Changes

  • Add SPVService which can fetch SPV client cell and BTC tx proof
  • Add SPV client cell to the cellDeps of the RGBPP tx and BTC time cells unlock tx
  • Add BTC tx proof to the witnesses of the RGBPP tx and BTC time cells unlock tx

Reviews

@Flouse @ShookLyngs @ahonn

@duanyytop duanyytop requested a review from ahonn March 14, 2024 13:37
packages/ckb/src/spv/index.ts Outdated Show resolved Hide resolved
rawTx.cellDeps.push(buildSpvClientCellDep(spvClient));

const rgbppUnlock = buildRgbppUnlockWitness(btcTxBytes, proof, ckbRawTx);
rawTx.witnesses = rawTx.witnesses.map((_) => rgbppUnlock);
Copy link
Contributor

@Flouse Flouse Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: I think only one witness is required for the current lock_script_group.
see https://github.com/ckb-cell/rgbpp/blob/e282e58af91511290309b4fca45ff59f759efad4/contracts/rgbpp-lock/src/main.rs#L97-L106

Copy link
Collaborator Author

@duanyytop duanyytop Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the RGBPP cells belong to the same ScriptGroup, because the RGBPP lock args must be different.

args: out_index | bitcoin_tx_id

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible that RGB++ cells are locked by the same btc utxo.
In that case, lock script of RGB++ cells or BTCTimeLock cells may be same, we should put lock witness only on the first position of the script group.

Comment on lines 40 to 51
const witnesses: Hex[] = inputs.map((_) => '0x');

for await (const cell of btcTimeCells) {
const { spvClient, proof } = await spvService.fetchSpvClientCellAndTxProof({
btcTxId: btcTxIdFromBtcTimeLockArgs(cell.output.lock.args),
confirmBlocks: BTC_JUMP_CONFIRMATION_BLOCKS,
});
cellDeps.push(buildSpvClientCellDep(spvClient));

const btcTimeUnlock = buildBtcTimeUnlockWitness(proof);
witnesses.push(btcTimeUnlock);
}
Copy link
Contributor

@Flouse Flouse Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: Only a single witness is required for the unlocking operation within the same script group.
See https://github.com/ckb-cell/rgbpp/blob/e282e58af91511290309b4fca45ff59f759efad4/contracts/btc-time-lock/src/main.rs#L58-L67

Copy link
Collaborator Author

@duanyytop duanyytop Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the BTC time cells belong to the same ScriptGroup, because the BTC time lock args are most likely different.

args: lock_script | after | bitcoin_tx_id

@Flouse Flouse self-requested a review March 14, 2024 15:10
Copy link
Contributor

@Flouse Flouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other implementations LGTM.

confirmBlocks,
}: SpvClientCellTxProofReq): Promise<SpvClientCellTxProofResponse> => {
let payload = {
id: 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The id parameter must be unique for each request following the JSON-RPC specification. Recommended to use a package such as uuid to generate a random ID for each request.

Copy link
Collaborator Author

@duanyytop duanyytop Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 97117c4

@Flouse Flouse requested a review from ShookLyngs March 15, 2024 09:29
@Flouse Flouse merged commit 9e9a055 into main Mar 15, 2024
1 check passed
@duanyytop duanyytop deleted the spv-client-proof branch March 15, 2024 09:30
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.

4 participants