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

Return tx instead of psbt #44

Merged
merged 18 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@babylonlabs-io/btc-staking-ts",
"version": "0.4.0-canary.2",
"version": "0.4.0-canary.3",
"description": "Library exposing methods for the creation and consumption of Bitcoin transactions pertaining to Babylon's Bitcoin Staking protocol.",
"module": "dist/index.js",
"main": "dist/index.cjs",
Expand Down
128 changes: 106 additions & 22 deletions src/staking/index.ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file requires formatting

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { networks, Transaction } from "bitcoinjs-lib";
import { networks, Psbt, Transaction } from "bitcoinjs-lib";
import { StakingParams } from "../types/params";
import { UTXO } from "../types/UTXO";
import { StakingScriptData, StakingScripts } from "./stakingScript";
Expand All @@ -21,8 +21,10 @@ import {
validateStakingTimelock,
validateStakingTxInputData,
} from "../utils/staking";
import { PsbtTransactionResult } from "../types/transaction";
import { PsbtResult, TransactionResult } from "../types/transaction";
import { toBuffers } from "../utils/staking";
import { stakingPsbt, unbondingPsbt } from "./psbt";
import { CovenantSignature } from "../types/covenantSignatures";
export * from "./stakingScript";

export interface StakerInfo {
Expand Down Expand Up @@ -117,13 +119,15 @@ export class Staking {
* @param {UTXO[]} inputUTXOs - The UTXOs to use as inputs for the staking
* transaction.
* @param {number} feeRate - The fee rate for the transaction in satoshis per byte.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {TransactionResult} - An object containing the unsigned transaction
* and fee
* @throws {StakingError} - If the transaction cannot be built
*/
public createStakingTransaction(
stakingAmountSat: number,
inputUTXOs: UTXO[],
feeRate: number,
): PsbtTransactionResult {
): TransactionResult {
validateStakingTxInputData(
stakingAmountSat,
this.stakingTimelock,
Expand All @@ -135,15 +139,28 @@ export class Staking {
const scripts = this.buildScripts();

try {
return stakingTransaction(
const { transaction, fee } = stakingTransaction(
scripts,
stakingAmountSat,
this.stakerInfo.address,
inputUTXOs,
this.network,
feeRate,
isTaproot(this.stakerInfo.address, this.network) ? Buffer.from(this.stakerInfo.publicKeyNoCoordHex, "hex") : undefined,
);
// Do a dry run of stakingPsbt to ensure the transaction can be converted to PSBT
jrwbabylonlab marked this conversation as resolved.
Show resolved Hide resolved
// with all the required properties before returning it
stakingPsbt(

Choose a reason for hiding this comment

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

when this would fail ?

Asking as tbh this weirds my out a bit. Imagine you are the caller of the function createStakingTransaction( and for some reason it throws some error about psbt. How do you supposed to handle it ?

When you call it you do not have even idea what is this PSBT thingy.

In my mind, the library code should build staking transaction and return it and all necessary data to the caller, and then it is totally up to the caller to convert this data into psbt when the caller want to do the signing.

Copy link
Collaborator Author

@jrwbabylonlab jrwbabylonlab Nov 25, 2024

Choose a reason for hiding this comment

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

I understand the concern, but I have a different perspective on this.

  • The dry run serves as an early validation step. If the transaction we create cannot be properly converted to PSBT later in the flow, it's better to fail fast rather than after the user has already spent gas fees on submitting EOI. (Especially the unbonding tx/psbt path )

  • Error msg should not be a concern. we could wrap it under try catch with custom error message and indicate the dry run in the method comment. The btc-staking-ts library's primary purpose is to facilitate BBN staking transactions. Its complete staking flow requires specific steps: building tx -> sending EOI -> converting to PSBT -> signing. Users implementing this library need to understand and follow this flow, hence error coming out dry run should be well understood.

  • From a responsibility perspective, if we output a transaction that fails during PSBT conversion after the user has already submitted EOI and paid gas fees, that would be a our responsibility. By validating PSBT compatibility upfront, we protect users from wasting resources on transactions that would ultimately fail.
    In essence, rather than viewing this as just a transaction creation method, we should consider it as part of a larger, well-defined staking flow where PSBT compatibility is a critical requirement.

Choose a reason for hiding this comment

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

I think what I am getting at is that our library should always output valid bitcoin transactions, and every valid bitcoin transaction is convertible into PSBT.

Therefore it goes that this check is a bit redundant, as if we create valid btc transactions it will also be psbt complainant. The only case it would not happen is if we were have some pretty bad bug.

It is not blocking for me (a bit of redundancy never hurts) but having psbt's here pollutes a bit core of the library (imo).

One note:

users implementing this library need to understand and follow this flow, hence error coming out dry run should be well understood.

In essence, rather than viewing this as just a transaction creation method, we should consider it as part of a larger, well-defined staking flow where PSBT compatibility is a critical requirement.

Imo this is idealist view of the world 😅 . In reality users of the library will use bits and pieces of the library which are most interesting to them. And hack around the rest.

The only way around it is to make library api surface as small as possible and as type controlled as possible, so that core operations happens on library custom types, and all conversions to other types happens on caller side (or in separate helpers provider by the library in some helper packages)

transaction,
this.network,
inputUTXOs,
isTaproot(
this.stakerInfo.address, this.network
) ? Buffer.from(this.stakerInfo.publicKeyNoCoordHex, "hex") : undefined,
);
return {
transaction,
fee,
};
} catch (error: unknown) {
throw StakingError.fromUnknown(
error, StakingErrorCode.BUILD_TRANSACTION_FAILURE,
Expand All @@ -152,16 +169,41 @@ export class Staking {
}
};

/**
* Create a staking psbt based on the existing staking transaction.
*
* @param {Transaction} stakingTx - The staking transaction.
* @param {UTXO[]} inputUTXOs - The UTXOs to use as inputs for the staking
* transaction. The UTXOs that were used to create the staking transaction should
* be included in this array.
* @returns {Psbt} - The psbt.
*/
public createStakingPsbt(
jrwbabylonlab marked this conversation as resolved.
Show resolved Hide resolved
stakingTx: Transaction,
inputUTXOs: UTXO[],
): Psbt {
return stakingPsbt(
stakingTx,
this.network,
inputUTXOs,
isTaproot(
this.stakerInfo.address, this.network
) ? Buffer.from(this.stakerInfo.publicKeyNoCoordHex, "hex") : undefined,
);
}

/**
* Create an unbonding transaction for staking.
*
* @param {Transaction} stakingTx - The staking transaction to unbond.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {TransactionResult} - An object containing the unsigned transaction
* and fee
* @throws {StakingError} - If the transaction cannot be built
*/
public createUnbondingTransaction(
stakingTx: Transaction,
) : PsbtTransactionResult {
covenantSigs?: CovenantSignature[],
) : TransactionResult {
// Build scripts
const scripts = this.buildScripts();

Expand All @@ -173,14 +215,25 @@ export class Staking {
)
// Create the unbonding transaction
try {
const { psbt } = unbondingTransaction(
const { transaction } = unbondingTransaction(
scripts,
stakingTx,
this.params.unbondingFeeSat,
this.network,
stakingOutputIndex,
);
return { psbt, fee: this.params.unbondingFeeSat };
// Do a dry run of unbondingPsbt to ensure the transaction can be converted to PSBT
// with all the required properties before returning it
unbondingPsbt(
scripts,
transaction,
stakingTx,
this.network,
);
return {
transaction,
fee: this.params.unbondingFeeSat,
};
} catch (error) {
throw StakingError.fromUnknown(
error, StakingErrorCode.BUILD_TRANSACTION_FAILURE,
Expand All @@ -190,29 +243,54 @@ export class Staking {
}

/**
* Create a withdrawal transaction that spends an unbonding transaction for observable staking.
* Create an unbonding psbt based on the existing unbonding transaction and
* staking transaction.
*
* @param {Transaction} unbondingTx - The unbonding transaction.
* @param {Transaction} stakingTx - The staking transaction.
*
* @returns {Psbt} - The psbt.
*/
public createUnbondingPsbt(
unbondingTx: Transaction,
stakingTx: Transaction,
): Psbt {
return unbondingPsbt(
this.buildScripts(),
unbondingTx,
stakingTx,
this.network,
);
}

/**
* Create a withdrawal transaction that spends an unbonding transaction.
*
* @param {Transaction} unbondingTx - The unbonding transaction to withdraw from.
* @param {number} feeRate - The fee rate for the transaction in satoshis per byte.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {PsbtResult} - An object containing the unsigned psbt and fee
* @throws {StakingError} - If the delegation is invalid or the transaction cannot be built
*/
public createWithdrawEarlyUnbondedTransaction (
unbondingTx: Transaction,
feeRate: number,
): PsbtTransactionResult {
): PsbtResult {
// Build scripts
const scripts = this.buildScripts();

// Create the withdraw early unbonded transaction
try {
return withdrawEarlyUnbondedTransaction(
const { psbt, fee } = withdrawEarlyUnbondedTransaction(
scripts,
unbondingTx,
this.stakerInfo.address,
this.network,
feeRate,
);
return {
psbt,
fee,
};
} catch (error) {
throw StakingError.fromUnknown(
error, StakingErrorCode.BUILD_TRANSACTION_FAILURE,
Expand All @@ -227,13 +305,13 @@ export class Staking {
*
* @param {Transaction} stakingTx - The staking transaction to withdraw from.
* @param {number} feeRate - The fee rate for the transaction in satoshis per byte.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {PsbtResult} - An object containing the unsigned psbt and fee
* @throws {StakingError} - If the delegation is invalid or the transaction cannot be built
*/
public createWithdrawTimelockUnbondedTransaction(
jrwbabylonlab marked this conversation as resolved.
Show resolved Hide resolved
stakingTx: Transaction,
feeRate: number,
): PsbtTransactionResult {
): PsbtResult {
// Build scripts
const scripts = this.buildScripts();

Expand Down Expand Up @@ -266,12 +344,12 @@ export class Staking {
* Create a slashing transaction spending from the staking output.
*
* @param {Transaction} stakingTx - The staking transaction to slash.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {PsbtResult} - An object containing the unsigned psbt and fee
* @throws {StakingError} - If the delegation is invalid or the transaction cannot be built
*/
public createStakingOutputSlashingTransaction(
stakingTx: Transaction,
) : PsbtTransactionResult {
) : PsbtResult {
if (!this.params.slashing) {
throw new StakingError(
StakingErrorCode.INVALID_PARAMS,
Expand All @@ -292,7 +370,10 @@ export class Staking {
this.params.slashing.minSlashingTxFeeSat,
this.network,
);
return { psbt, fee: this.params.slashing.minSlashingTxFeeSat };
return {
psbt,
fee: this.params.slashing.minSlashingTxFeeSat,
};
} catch (error) {
throw StakingError.fromUnknown(
error, StakingErrorCode.BUILD_TRANSACTION_FAILURE,
Expand All @@ -305,12 +386,12 @@ export class Staking {
* Create a slashing transaction for an unbonding output.
*
* @param {Transaction} unbondingTx - The unbonding transaction to slash.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {PsbtResult} - An object containing the unsigned psbt and fee
* @throws {StakingError} - If the delegation is invalid or the transaction cannot be built
*/
public createUnbondingOutputSlashingTransaction(
unbondingTx: Transaction,
): PsbtTransactionResult {
): PsbtResult {
if (!this.params.slashing) {
throw new StakingError(
StakingErrorCode.INVALID_PARAMS,
Expand All @@ -330,7 +411,10 @@ export class Staking {
this.params.slashing.minSlashingTxFeeSat,
this.network,
);
return { psbt, fee: this.params.slashing.minSlashingTxFeeSat };
return {
psbt,
fee: this.params.slashing.minSlashingTxFeeSat,
};
} catch (error) {
throw StakingError.fromUnknown(
error, StakingErrorCode.BUILD_TRANSACTION_FAILURE,
Expand Down
48 changes: 42 additions & 6 deletions src/staking/observable/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import { StakingError, StakingErrorCode } from "../../error";
import { stakingTransaction } from "../transactions";
import { isTaproot } from "../../utils/btc";
import { toBuffers, validateStakingTxInputData } from "../../utils/staking";
import { PsbtTransactionResult } from "../../types/transaction";
import { TransactionResult } from "../../types/transaction";
import { ObservableStakingScriptData, ObservableStakingScripts } from "./observableStakingScript";
import { StakerInfo, Staking } from "..";
import { networks } from "bitcoinjs-lib";
import { networks, Psbt, Transaction } from "bitcoinjs-lib";
import { stakingPsbt, unbondingPsbt } from "../psbt";
import { CovenantSignature } from "../../types/covenantSignatures";
jrwbabylonlab marked this conversation as resolved.
Show resolved Hide resolved
export * from "./observableStakingScript";

/**
Expand Down Expand Up @@ -104,13 +106,14 @@ export class ObservableStaking extends Staking {
* @param {UTXO[]} inputUTXOs - The UTXOs to use as inputs for the staking
* transaction.
* @param {number} feeRate - The fee rate for the transaction in satoshis per byte.
* @returns {PsbtTransactionResult} - An object containing the unsigned psbt and fee
* @returns {TransactionResult} - An object containing the unsigned transaction,
* psbt, and fee
*/
public createStakingTransaction(
stakingAmountSat: number,
inputUTXOs: UTXO[],
feeRate: number,
): PsbtTransactionResult{
): TransactionResult {
validateStakingTxInputData(
stakingAmountSat,
this.stakingTimelock,
Expand All @@ -123,25 +126,58 @@ export class ObservableStaking extends Staking {

// Create the staking transaction
try {
return stakingTransaction(
const { transaction, fee } = stakingTransaction(
scripts,
stakingAmountSat,
this.stakerInfo.address,
inputUTXOs,
this.network,
feeRate,
isTaproot(this.stakerInfo.address, this.network) ? Buffer.from(this.stakerInfo.publicKeyNoCoordHex, "hex") : undefined,
// `lockHeight` is exclusive of the provided value.
// For example, if a Bitcoin height of X is provided,
// the transaction will be included starting from height X+1.
// https://learnmeabitcoin.com/technical/transaction/locktime/
this.params.activationHeight - 1,
);

stakingPsbt(
transaction,
this.network,
inputUTXOs,
isTaproot(
this.stakerInfo.address, this.network
) ? Buffer.from(this.stakerInfo.publicKeyNoCoordHex, "hex") : undefined,
);

return {
transaction,
fee,
};
} catch (error: unknown) {
throw StakingError.fromUnknown(
error, StakingErrorCode.BUILD_TRANSACTION_FAILURE,
"Cannot build unsigned staking transaction",
);
}
};

/**
* Create an unbonding psbt based on the existing unbonding transaction and
* staking transaction.
*
* @param {Transaction} unbondingTx - The unbonding transaction.
* @param {Transaction} stakingTx - The staking transaction.
* @returns {Psbt} - The psbt.
*/
public createUnbondingPsbt(
jrwbabylonlab marked this conversation as resolved.
Show resolved Hide resolved
unbondingTx: Transaction,
stakingTx: Transaction,
): Psbt {
return unbondingPsbt(
this.buildScripts(),
unbondingTx,
stakingTx,
this.network,
);
}
}
Loading