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.

Sign up for GitHub

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(jellyfish-transaction-builder): Extend createDeFiTx to allow for custom vins and vouts #1892

Closed
wants to merge 1 commit into from
Closed

feat(jellyfish-transaction-builder): Extend createDeFiTx to allow for custom vins and vouts #1892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 44 additions & 2 deletions packages/jellyfish-transaction-builder/__tests__/txn/txn_builder_p2wpkh.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import BigNumber from 'bignumber.js'
import { MasterNodeRegTestContainer } from '@defichain/testcontainers'
import { getProviders, MockProviders } from '../provider.mock'
import { P2WPKHTxnBuilder } from '../../src'
import { P2WPKHTxnBuilder, Prevout } from '../../src'
import { fundEllipticPair } from '../test.utils'
import { CDfTx, DeFiOpUnmapped, OP_CODES, OP_DEFI_TX } from '@defichain/jellyfish-transaction'
import { CDfTx, DeFiOpUnmapped, OP_CODES, OP_DEFI_TX, Vin, Vout } from '@defichain/jellyfish-transaction'
import { RegTest } from '@defichain/jellyfish-network'

// P2WPKHTxnBuilder is abstract and not instantiable
Expand Down Expand Up @@ -92,4 +92,46 @@ describe('createDeFiTx()', () => {
expect(result.vout[1].value.gt(new BigNumber(99.999).minus(spendAmount))).toBeTruthy()
expect(result.vout[1].value.lt(new BigNumber(100).minus(spendAmount))).toBeTruthy()
})

it('should create DfTx stack correctly with custom vin and vout', async () => {
const script = await providers.elliptic.script()

const collateralPrevout: Prevout = {
txid: '1111111122222222333333334444444455555555666666667777777788888888',
vout: 0,
script: script,
value: new BigNumber(15),
tokenId: 0
}
const collateralVout: Vout = {
script: script,
value: new BigNumber(15),
tokenId: 0
}
const collateralVin: Vin = {
txid: '1111111122222222333333334444444455555555666666667777777788888888',
index: 1,
script: { stack: [] },
sequence: 0xffffffff
}
const customVinVout = {
prevout: collateralPrevout,
vin: collateralVin,
vout: collateralVout
}

const result = await builder.createDeFiTx(dummyDfTx, script, undefined, [customVinVout])

expect(result.vin.length).toStrictEqual(8)
expect(result.vout.length).toStrictEqual(3)

expect(result.vout[0].value).toStrictEqual(new BigNumber(0))
expect(result.vout[1]).toStrictEqual(expect.objectContaining({
value: new BigNumber(15),
script: script
}))
expect(result.vout[2].value.gt(99.999)).toBeTruthy()
expect(result.vout[2].value.lt(100)).toBeTruthy()
expect(result.vout[2].script).toStrictEqual(script)
})
})
16 changes: 12 additions & 4 deletions packages/jellyfish-transaction-builder/src/txn/txn_builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export abstract class P2WPKHTxnBuilder {
* Craft a transaction with OP_DEFI_TX from the output of OP_CODES.OP_DEFI_TX_.
* This is a helper method for creating custom defi transactions.
*
* Optionally, allow for custom inputs with vin and vout.
*
* As DeFi custom transaction will always require small amount of DFI,
* collectPrevouts() is set to search for at least 0.001 DFI amount of prevout.
* This will also evidently merge small prevout during the operation.
Expand All @@ -98,15 +100,21 @@ export abstract class P2WPKHTxnBuilder {
* @param {OP_DEFI_TX} opDeFiTx to create
* @param {Script} changeScript to send unspent to after deducting the fees
* @param {BigNumber} [outValue=0] for the opDeFiTx, usually always be 0.
* @param {Array<{ vin: Vin, vout: Vout, prevout: Prevout }>} [customVinVout = []] for custom vin and vout
*/
async createDeFiTx (
opDeFiTx: OP_DEFI_TX,
changeScript: Script,
outValue: BigNumber = new BigNumber('0')
outValue: BigNumber = new BigNumber('0'),
customVinVout: Array<{ vin: Vin, vout: Vout, prevout: Prevout }> = []
Copy link
Contributor

Choose a reason for hiding this comment

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

This overloader concept isn't really applicable and it doesn't gel well.

  • Given that can inject any Vin, the Vin can already be collected by await this.collectPrevouts(minFee) and included in vins, giving you an invalid Transaction due to duplicated Vins.
  • The assumption is with the Vin provided that you have the signing rights to it.
  • That {Vin, Vout, Prevout} comes in a Set, that's not true since only Vin is tied to Prevout. The amount Vin to Vout ratio is not exclusive. So an array won't make sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

We also have to ensure collectPrevout doesn't collect the Vin you need to sign in a special way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your setup requires:

  1. A custom "Prevouts Collector" implementation since you need control of the Vins
  2. Injection of Vout in a special order.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense for the upstream implementation to just prepare their Transaction from scratch?

We change protected async sign to public sync sign to allow that. And anything else that is required we make it public or create an abstraction for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, a little lost 😓 correct me if I'm wrong:

  • I will implement a "Prevouts Collector" as a method in P2WPKHTxnBuilder class
  • then combine the outputs of it along with outputs of await this.collectPrevouts(minFee) so Vin and Vout can be placed in the correct order that doesn't have duplicates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prepare their Transaction from scratch

do you mean build the transaction from scratch with vin and vout and then sign it, all in the updatemasternode function in packages/jellyfish-transaction-builder/src/txn/txn_builder_masternode.ts?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, that's right. Since your requirements require vin and vout to be arranged in a certain way. Nobody else will probably need it. It is better to keep the custom logic there instead of bleeding it into an abstraction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that makes sense. I will do it this way since no other functions need it 👍

): Promise<TransactionSegWit> {
const minFee = outValue.plus(0.001) // see JSDoc above
const { prevouts, vin, total } = await this.collectPrevouts(minFee)

const customVins = customVinVout.map(({ vin }) => vin)
const customPrevouts = customVinVout.map(({ prevout }) => prevout)
const customVouts = customVinVout.map(({ vout }) => vout)

const deFiOut: Vout = {
value: outValue,
script: {
Expand All @@ -123,15 +131,15 @@ export abstract class P2WPKHTxnBuilder {

const txn: Transaction = {
version: DeFiTransactionConstants.Version,
vin: vin,
vout: [deFiOut, change],
vin: [...vin, ...customVins],
vout: [deFiOut, ...customVouts, change],
lockTime: 0x00000000
}

const fee = await this.calculateFee(txn)
change.value = total.minus(outValue).minus(fee)

return await this.sign(txn, prevouts)
return await this.sign(txn, [...prevouts, ...customPrevouts])
}

/**
Expand Down