-
Notifications
You must be signed in to change notification settings - Fork 295
feat(sdk-coin-sui): fee payer signing support #6423
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,10 +6,10 @@ import { | |
Signature, | ||
TransactionType as BitGoTransactionType, | ||
} from '@bitgo/sdk-core'; | ||
import { SuiProgrammableTransaction, SuiTransaction, SuiTransactionType, TxData } from './iface'; | ||
import { SuiProgrammableTransaction, SuiTransaction, SuiTransactionType, TxData, GasData } from './iface'; | ||
import { BaseCoin as CoinConfig } from '@bitgo/statics'; | ||
import utils, { AppId, Intent, IntentScope, IntentVersion, isImmOrOwnedObj } from './utils'; | ||
import { GasData, normalizeSuiAddress, normalizeSuiObjectId, SuiObjectRef } from './mystenlab/types'; | ||
import { normalizeSuiAddress, normalizeSuiObjectId, SuiObjectRef } from './mystenlab/types'; | ||
import { SIGNATURE_SCHEME_BYTES } from './constants'; | ||
import { Buffer } from 'buffer'; | ||
import { fromB64, toB64 } from '@mysten/bcs'; | ||
|
@@ -20,10 +20,12 @@ import { builder, MergeCoinsTransaction, TransactionType } from './mystenlab/bui | |
import blake2b from '@bitgo/blake2b'; | ||
import { hashTypedData } from './mystenlab/cryptography/hash'; | ||
|
||
export abstract class Transaction<T> extends BaseTransaction { | ||
export abstract class Transaction<T = SuiProgrammableTransaction> extends BaseTransaction { | ||
protected _suiTransaction: SuiTransaction<T>; | ||
protected _signature: Signature; | ||
protected _feePayerSignature: Signature; | ||
private _serializedSig: Uint8Array; | ||
private _serializedFeePayerSig: Uint8Array; | ||
|
||
protected constructor(_coinConfig: Readonly<CoinConfig>) { | ||
super(_coinConfig); | ||
|
@@ -48,17 +50,31 @@ export abstract class Transaction<T> extends BaseTransaction { | |
addSignature(publicKey: BasePublicKey, signature: Buffer): void { | ||
this._signatures.push(signature.toString('hex')); | ||
this._signature = { publicKey, signature }; | ||
this.setSerializedSig(publicKey, signature); | ||
this.serialize(); | ||
} | ||
|
||
addFeePayerSignature(publicKey: BasePublicKey, signature: Buffer): void { | ||
this._feePayerSignature = { publicKey, signature }; | ||
this.setSerializedFeePayerSig(publicKey, signature); | ||
} | ||
|
||
get suiSignature(): Signature { | ||
return this._signature; | ||
} | ||
|
||
get feePayerSignature(): Signature { | ||
return this._feePayerSignature; | ||
} | ||
|
||
get serializedSig(): Uint8Array { | ||
return this._serializedSig; | ||
} | ||
|
||
get serializedFeePayerSig(): Uint8Array { | ||
return this._serializedFeePayerSig; | ||
} | ||
|
||
setSerializedSig(publicKey: BasePublicKey, signature: Buffer): void { | ||
const pubKey = Buffer.from(publicKey.pub, 'hex'); | ||
const serialized_sig = new Uint8Array(1 + signature.length + pubKey.length); | ||
|
@@ -68,6 +84,15 @@ export abstract class Transaction<T> extends BaseTransaction { | |
this._serializedSig = serialized_sig; | ||
} | ||
|
||
setSerializedFeePayerSig(publicKey: BasePublicKey, signature: Buffer): void { | ||
const pubKey = Buffer.from(publicKey.pub, 'hex'); | ||
const serialized_sig = new Uint8Array(1 + signature.length + pubKey.length); | ||
serialized_sig.set(SIGNATURE_SCHEME_BYTES); | ||
serialized_sig.set(signature, 1); | ||
serialized_sig.set(pubKey, 1 + signature.length); | ||
this._serializedFeePayerSig = serialized_sig; | ||
} | ||
|
||
/** @inheritdoc */ | ||
canSign(key: BaseKey): boolean { | ||
return true; | ||
|
@@ -78,7 +103,6 @@ export abstract class Transaction<T> extends BaseTransaction { | |
* | ||
* @param {KeyPair} signer key | ||
*/ | ||
|
||
sign(signer: KeyPair): void { | ||
if (!this._suiTransaction) { | ||
throw new InvalidTransactionError('empty transaction to sign'); | ||
|
@@ -87,16 +111,56 @@ export abstract class Transaction<T> extends BaseTransaction { | |
const intentMessage = this.signablePayload; | ||
const signature = signer.signMessageinUint8Array(intentMessage); | ||
|
||
this.setSerializedSig({ pub: signer.getKeys().pub }, Buffer.from(signature)); | ||
this.addSignature({ pub: signer.getKeys().pub }, Buffer.from(signature)); | ||
} | ||
|
||
/** | ||
* Sign this transaction as a fee payer | ||
* | ||
* @param {KeyPair} signer key | ||
*/ | ||
signFeePayer(signer: KeyPair): void { | ||
if (!this._suiTransaction) { | ||
throw new InvalidTransactionError('empty transaction to sign'); | ||
} | ||
|
||
if ( | ||
!this._suiTransaction.gasData || | ||
!('sponsor' in this._suiTransaction.gasData) || | ||
!this._suiTransaction.gasData.sponsor | ||
) { | ||
throw new InvalidTransactionError('transaction does not have a fee payer'); | ||
} | ||
|
||
const intentMessage = this.signablePayload; | ||
const signature = signer.signMessageinUint8Array(intentMessage); | ||
|
||
this.addFeePayerSignature({ pub: signer.getKeys().pub }, Buffer.from(signature)); | ||
} | ||
|
||
/** @inheritdoc */ | ||
toBroadcastFormat(): string { | ||
if (!this._suiTransaction) { | ||
throw new InvalidTransactionError('Empty transaction'); | ||
} | ||
return this.serialize(); | ||
|
||
if (!this._serializedSig) { | ||
throw new InvalidTransactionError('Transaction must be signed'); | ||
} | ||
|
||
const result = { | ||
txBytes: this.serialize(), | ||
senderSignature: toB64(this._serializedSig), | ||
}; | ||
|
||
if (this._suiTransaction.gasData?.sponsor) { | ||
if (!this._serializedFeePayerSig) { | ||
throw new InvalidTransactionError('Sponsored transaction must have fee payer signature'); | ||
} | ||
result['sponsorSignature'] = toB64(this._serializedFeePayerSig); | ||
} | ||
|
||
return JSON.stringify(result); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The toBroadcastFormat method now returns JSON instead of the previous string format. This appears to be a breaking change that could affect existing consumers of this API. Ensure this change is intentional and properly documented as a breaking change. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
} | ||
|
||
/** @inheritdoc */ | ||
|
@@ -165,6 +229,19 @@ export abstract class Transaction<T> extends BaseTransaction { | |
const inputs = transactionBlock.inputs.map((txInput) => txInput.value); | ||
const transactions = transactionBlock.transactions; | ||
const txType = this.getSuiTransactionType(transactions); | ||
|
||
const gasData: GasData = { | ||
payment: this.normalizeCoins(transactionBlock.gasConfig.payment!), | ||
owner: normalizeSuiAddress(transactionBlock.gasConfig.owner!), | ||
price: Number(transactionBlock.gasConfig.price as string), | ||
budget: Number(transactionBlock.gasConfig.budget as string), | ||
}; | ||
|
||
// Only add sponsor if it exists | ||
if (transactionBlock.gasConfig.sponsor) { | ||
gasData.sponsor = normalizeSuiAddress(transactionBlock.gasConfig.sponsor); | ||
} | ||
|
||
return { | ||
id: transactionBlock.getDigest(), | ||
type: txType, | ||
|
@@ -173,12 +250,7 @@ export abstract class Transaction<T> extends BaseTransaction { | |
inputs: inputs, | ||
transactions: transactions, | ||
}, | ||
gasData: { | ||
payment: this.normalizeCoins(transactionBlock.gasConfig.payment!), | ||
owner: normalizeSuiAddress(transactionBlock.gasConfig.owner!), | ||
price: Number(transactionBlock.gasConfig.price as string), | ||
budget: Number(transactionBlock.gasConfig.budget as string), | ||
}, | ||
gasData: gasData, | ||
}; | ||
} | ||
|
||
|
@@ -213,12 +285,20 @@ export abstract class Transaction<T> extends BaseTransaction { | |
} | ||
|
||
static getProperGasData(k: any): GasData { | ||
return { | ||
payment: [this.normalizeSuiObjectRef(k.gasData.payment)], | ||
const gasData: GasData = { | ||
payment: Array.isArray(k.gasData.payment) | ||
? k.gasData.payment.map((p: any) => this.normalizeSuiObjectRef(p)) | ||
: [this.normalizeSuiObjectRef(k.gasData.payment)], | ||
owner: utils.normalizeHexId(k.gasData.owner), | ||
price: Number(k.gasData.price), | ||
budget: Number(k.gasData.budget), | ||
}; | ||
|
||
if (k.gasData.sponsor) { | ||
gasData.sponsor = utils.normalizeHexId(k.gasData.sponsor); | ||
} | ||
|
||
return gasData; | ||
} | ||
|
||
private static normalizeCoins(coins: any[]): SuiObjectRef[] { | ||
|
@@ -267,4 +347,15 @@ export abstract class Transaction<T> extends BaseTransaction { | |
|
||
return inputGasPaymentObjects; | ||
} | ||
|
||
hasFeePayerSig(): boolean { | ||
return this._feePayerSignature !== undefined; | ||
} | ||
|
||
getFeePayerPubKey(): string | undefined { | ||
if (!this._feePayerSignature || !this._feePayerSignature.publicKey) { | ||
return undefined; | ||
} | ||
return this._feePayerSignature.publicKey.pub; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,10 +14,10 @@ import { Transaction } from './transaction'; | |||||||||||||
import utils from './utils'; | ||||||||||||||
import BigNumber from 'bignumber.js'; | ||||||||||||||
import { BaseCoin as CoinConfig } from '@bitgo/statics'; | ||||||||||||||
import { SuiProgrammableTransaction, SuiTransactionType } from './iface'; | ||||||||||||||
import { GasData, SuiProgrammableTransaction, SuiTransactionType } from './iface'; | ||||||||||||||
import { DUMMY_SUI_GAS_PRICE } from './constants'; | ||||||||||||||
import { KeyPair } from './keyPair'; | ||||||||||||||
import { GasData, SuiObjectRef } from './mystenlab/types'; | ||||||||||||||
import { SuiObjectRef } from './mystenlab/types'; | ||||||||||||||
|
||||||||||||||
export abstract class TransactionBuilder<T = SuiProgrammableTransaction> extends BaseTransactionBuilder { | ||||||||||||||
protected _transaction: Transaction<T>; | ||||||||||||||
|
@@ -52,7 +52,34 @@ export abstract class TransactionBuilder<T = SuiProgrammableTransaction> extends | |||||||||||||
protected signImplementation(key: BaseKey): Transaction<T> { | ||||||||||||||
const signer = new KeyPair({ prv: key.key }); | ||||||||||||||
this._signer = signer; | ||||||||||||||
this.transaction.sign(signer); | ||||||||||||||
const signable = this.transaction.signablePayload; | ||||||||||||||
const signature = signer.signMessageinUint8Array(signable); | ||||||||||||||
const signatureBuffer = Buffer.from(signature); | ||||||||||||||
this.transaction.addSignature({ pub: signer.getKeys().pub }, signatureBuffer); | ||||||||||||||
this.transaction.setSerializedSig({ pub: signer.getKeys().pub }, signatureBuffer); | ||||||||||||||
Comment on lines
+55
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The signImplementation method has been modified to duplicate signing logic that already exists in the Transaction.sign() method. This creates code duplication and potential maintenance issues. Consider calling this.transaction.sign(signer) instead of reimplementing the signing logic.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||
return this.transaction; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Signs the transaction as a fee payer. | ||||||||||||||
* | ||||||||||||||
* @param {BaseKey} key - The private key to sign the transaction with. | ||||||||||||||
* @returns {Transaction<T>} - The signed transaction. | ||||||||||||||
*/ | ||||||||||||||
signFeePayer(key: BaseKey): Transaction<T> { | ||||||||||||||
this.validateKey(key); | ||||||||||||||
|
||||||||||||||
// Check if gasData exists and has a sponsor | ||||||||||||||
if (!this._gasData?.sponsor) { | ||||||||||||||
throw new BuildTransactionError('Transaction must have a fee payer (sponsor) to sign as fee payer'); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
const signer = new KeyPair({ prv: key.key }); | ||||||||||||||
const signable = this.transaction.signablePayload; | ||||||||||||||
const signature = signer.signMessageinUint8Array(signable); | ||||||||||||||
const signatureBuffer = Buffer.from(signature); | ||||||||||||||
this.transaction.addFeePayerSignature({ pub: signer.getKeys().pub }, signatureBuffer); | ||||||||||||||
|
||||||||||||||
return this.transaction; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
|
@@ -87,6 +114,30 @@ export abstract class TransactionBuilder<T = SuiProgrammableTransaction> extends | |||||||||||||
return this; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Sets the gas sponsor (fee payer) address for this transaction. | ||||||||||||||
* When specified, the sponsor will be responsible for paying transaction fees. | ||||||||||||||
* | ||||||||||||||
* @param {string} sponsorAddress the account that will pay for this transaction | ||||||||||||||
* @returns {TransactionBuilder} This transaction builder | ||||||||||||||
*/ | ||||||||||||||
sponsor(sponsorAddress: string): this { | ||||||||||||||
if (!utils.isValidAddress(sponsorAddress)) { | ||||||||||||||
throw new BuildTransactionError('Invalid or missing sponsor, got: ' + sponsorAddress); | ||||||||||||||
} | ||||||||||||||
if (!this._gasData) { | ||||||||||||||
throw new BuildTransactionError('gasData must be set before setting sponsor'); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Update the gasData with the sponsor | ||||||||||||||
this._gasData = { | ||||||||||||||
...this._gasData, | ||||||||||||||
sponsor: sponsorAddress, | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
return this; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Initialize the transaction builder fields using the decoded transaction data | ||||||||||||||
* | ||||||||||||||
|
@@ -117,6 +168,14 @@ export abstract class TransactionBuilder<T = SuiProgrammableTransaction> extends | |||||||||||||
if (!utils.isValidAddress(gasData.owner)) { | ||||||||||||||
throw new BuildTransactionError('Invalid gas address ' + gasData.owner); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Validate sponsor address if present | ||||||||||||||
if ('sponsor' in gasData && gasData.sponsor !== undefined) { | ||||||||||||||
if (!utils.isValidAddress(gasData.sponsor)) { | ||||||||||||||
throw new BuildTransactionError('Invalid sponsor address ' + gasData.sponsor); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
this.validateGasPayment(gasData.payment); | ||||||||||||||
this.validateGasBudget(gasData.budget); | ||||||||||||||
this.validateGasPrice(gasData.price); | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addSignature method now calls setSerializedSig, but this creates a dependency where addSignature must always be called after setSerializedSig is available. This could cause issues if the calling order changes. Consider whether this logic belongs in addSignature or if the serialization should be handled separately.
Copilot uses AI. Check for mistakes.