From 26595ab3acb177d2c40470d01144d283a0598ca7 Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Wed, 7 Sep 2022 19:08:27 -0500 Subject: [PATCH] Fix bug where no exception was thrown if the acceptor did not have enough money (#4728) * Fix bug where no exception was thrown if the acceptor did not have enough money * Revert ContractInfo.max definition, fix bug in acceptDLCOffer * Fix incorrect usages of ContractInfo.max * Refactor ContractInfo.max -> ContractInfo.maxOfferorPayout * Fix dlc.md --- .../org/bitcoins/commons/DLCStatusTest.scala | 16 +++---- .../protocol/dlc/build/DLCTxBuilder.scala | 6 ++- .../protocol/dlc/models/ContractInfo.scala | 16 ++++--- .../org/bitcoins/dlc/testgen/DLCTLVGen.scala | 2 +- .../dlc/wallet/DLCWalletCallbackTest.scala | 4 -- .../dlc/wallet/WalletDLCSetupTest.scala | 39 ++++++++++++++-- .../org/bitcoins/dlc/wallet/DLCWallet.scala | 3 +- docs/core/dlc.md | 2 +- .../testkit/wallet/DLCWalletUtil.scala | 45 ++++++++++++++++--- 9 files changed, 101 insertions(+), 32 deletions(-) diff --git a/app-commons-test/src/test/scala/org/bitcoins/commons/DLCStatusTest.scala b/app-commons-test/src/test/scala/org/bitcoins/commons/DLCStatusTest.scala index fbb8233def..f4ce3969c4 100644 --- a/app-commons-test/src/test/scala/org/bitcoins/commons/DLCStatusTest.scala +++ b/app-commons-test/src/test/scala/org/bitcoins/commons/DLCStatusTest.scala @@ -25,7 +25,7 @@ class DLCStatusTest extends BitcoinSJvmTest { case (isInit, offerTLV) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val payoutAddress = Option.empty[PayoutAddress] @@ -60,7 +60,7 @@ class DLCStatusTest extends BitcoinSJvmTest { case (isInit, offerTLV, contractId) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral // random testnet address val payoutAddress = @@ -102,7 +102,7 @@ class DLCStatusTest extends BitcoinSJvmTest { case (isInit, offerTLV, contractId, txId) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val payoutAddress = Option.empty[PayoutAddress] @@ -140,7 +140,7 @@ class DLCStatusTest extends BitcoinSJvmTest { case (isInit, offerTLV, contractId, fundingTxId) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val payoutAddress = Option.empty[PayoutAddress] @@ -178,7 +178,7 @@ class DLCStatusTest extends BitcoinSJvmTest { case (isInit, offerTLV, contractId, fundingTxId) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val payoutAddress = Option.empty[PayoutAddress] @@ -219,7 +219,7 @@ class DLCStatusTest extends BitcoinSJvmTest { ) { case (isInit, offerTLV, contractId, fundingTxId, closingTxId, sigs) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val randomMyPayout = Math.abs(scala.util.Random.nextLong() % totalCollateral.toLong) val myPayout: CurrencyUnit = Satoshis(randomMyPayout) @@ -274,7 +274,7 @@ class DLCStatusTest extends BitcoinSJvmTest { ) { case (isInit, offerTLV, contractId, fundingTxId, closingTxId, sig) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val randomMyPayout = Math.abs(scala.util.Random.nextLong() % totalCollateral.toLong) @@ -330,7 +330,7 @@ class DLCStatusTest extends BitcoinSJvmTest { ) { case (isInit, offerTLV, contractId, fundingTxId, closingTxId) => val offer = DLCOffer.fromTLV(offerTLV) - val totalCollateral = offer.contractInfo.max + val totalCollateral = offer.contractInfo.totalCollateral val randomMyPayout = Math.abs(scala.util.Random.nextLong() % totalCollateral.toLong) diff --git a/core/src/main/scala/org/bitcoins/core/protocol/dlc/build/DLCTxBuilder.scala b/core/src/main/scala/org/bitcoins/core/protocol/dlc/build/DLCTxBuilder.scala index 4ff5e4e5c4..4f46d9e294 100644 --- a/core/src/main/scala/org/bitcoins/core/protocol/dlc/build/DLCTxBuilder.scala +++ b/core/src/main/scala/org/bitcoins/core/protocol/dlc/build/DLCTxBuilder.scala @@ -103,8 +103,10 @@ case class DLCTxBuilder(offer: DLCOffer, accept: DLCAcceptWithoutSigs) { "Offer change address must have same network as final address") require(acceptChangeAddress.networkParameters == network, "Accept change address must have same network as final address") - require(totalInput >= contractInfo.max, - "Total collateral must add up to max winnings") + require( + totalInput >= contractInfo.totalCollateral, + s"Total collateral must add up to max winnings, totalInput=${totalInput} contractInfo.totalCollateral=${contractInfo.totalCollateral}" + ) require( offerTotalFunding >= offerTotalCollateral, "Offer funding inputs must add up to at least offer's total collateral") diff --git a/core/src/main/scala/org/bitcoins/core/protocol/dlc/models/ContractInfo.scala b/core/src/main/scala/org/bitcoins/core/protocol/dlc/models/ContractInfo.scala index 2555581235..4cb5fa79ce 100644 --- a/core/src/main/scala/org/bitcoins/core/protocol/dlc/models/ContractInfo.scala +++ b/core/src/main/scala/org/bitcoins/core/protocol/dlc/models/ContractInfo.scala @@ -42,7 +42,7 @@ sealed trait ContractInfo extends TLVSerializable[ContractInfoTLV] { def contractDescriptors: Vector[ContractDescriptor] /** Returns the maximum payout the offerer could win from this contract */ - def max: Satoshis + def maxOffererPayout: Satoshis /** Computes the CET set and their corresponding payouts using CETCalculator. */ def allOutcomesAndPayouts: Vector[(OracleOutcome, Satoshis)] @@ -220,10 +220,13 @@ case class SingleContractInfo( } /** @inheritdoc */ - override val max: Satoshis = contractDescriptor match { - case descriptor: EnumContractDescriptor => - descriptor.values.maxBy(_.toLong) - case _: NumericContractDescriptor => totalCollateral + override val maxOffererPayout: Satoshis = { + contractDescriptor match { + case descriptor: EnumContractDescriptor => + descriptor.values.maxBy(_.toLong) + case _: NumericContractDescriptor => + totalCollateral + } } /** @inheritdoc */ @@ -403,7 +406,8 @@ case class DisjointUnionContractInfo(contracts: Vector[SingleContractInfo]) } /** @inheritdoc */ - override val max: Satoshis = contracts.map(_.max).max + override val maxOffererPayout: Satoshis = + contracts.map(_.maxOffererPayout).max /** @inheritdoc */ override lazy val allOutcomesAndPayouts: Vector[(OracleOutcome, Satoshis)] = { diff --git a/dlc-test/src/test/scala/org/bitcoins/dlc/testgen/DLCTLVGen.scala b/dlc-test/src/test/scala/org/bitcoins/dlc/testgen/DLCTLVGen.scala index eaff9b44bc..f59bab27e9 100644 --- a/dlc-test/src/test/scala/org/bitcoins/dlc/testgen/DLCTLVGen.scala +++ b/dlc-test/src/test/scala/org/bitcoins/dlc/testgen/DLCTLVGen.scala @@ -405,7 +405,7 @@ object DLCTLVGen { changeAddress: BitcoinAddress = address(), changeSerialId: UInt64 = DLCMessage.genSerialId()): DLCAccept = { val totalCollateral = - offer.contractInfo.max - offer.collateral + overCollateral + offer.contractInfo.maxOffererPayout - offer.collateral + overCollateral val cetSignatures = cetSigs( diff --git a/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/DLCWalletCallbackTest.scala b/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/DLCWalletCallbackTest.scala index 9a17a125b4..90788ccc8c 100644 --- a/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/DLCWalletCallbackTest.scala +++ b/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/DLCWalletCallbackTest.scala @@ -3,7 +3,6 @@ package org.bitcoins.dlc.wallet import org.bitcoins.core.protocol.dlc.models.{ DLCState, DLCStatus, - DisjointUnionContractInfo, SingleContractInfo } import org.bitcoins.testkit.wallet.FundWalletUtil.FundedDLCWallet @@ -101,9 +100,6 @@ class DLCWalletCallbackTest extends BitcoinSDualWalletTest { DLCWalletUtil.sampleContractInfo match { case single: SingleContractInfo => DLCWalletUtil.getSigs(single) - case disjoint: DisjointUnionContractInfo => - sys.error( - s"Cannot retrieve sigs for disjoint union contract, got=$disjoint") } } transaction <- walletA.executeDLC(contractId, sigs._1).map(_.get) diff --git a/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/WalletDLCSetupTest.scala b/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/WalletDLCSetupTest.scala index 19efde1434..5e15710735 100644 --- a/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/WalletDLCSetupTest.scala +++ b/dlc-wallet-test/src/test/scala/org/bitcoins/dlc/wallet/WalletDLCSetupTest.scala @@ -76,7 +76,8 @@ class WalletDLCSetupTest extends BitcoinSDualWalletTest { accept.fundingInputs .map(_.output.value) .sum >= accept.collateral) - assert(accept.collateral == offer.contractInfo.max - offer.collateral) + assert( + accept.collateral == offer.contractInfo.totalCollateral - offer.collateral) assert(accept.changeAddress.value.nonEmpty) } @@ -143,6 +144,12 @@ class WalletDLCSetupTest extends BitcoinSDualWalletTest { testNegotiate(fundedDLCWallets, DLCWalletUtil.sampleDLCOffer) } + it must "correctly negotiate a non winner take all dlc" in { + fundedDLCWallets: (FundedDLCWallet, FundedDLCWallet) => + testNegotiate(fundedDLCWallets, + DLCWalletUtil.sampleDLCOfferNonWinnerTakeAll) + } + it must "correctly negotiate a dlc with a multi-nonce oracle info" in { fundedDLCWallets: (FundedDLCWallet, FundedDLCWallet) => testNegotiate(fundedDLCWallets, DLCWalletUtil.sampleMultiNonceDLCOffer) @@ -310,7 +317,8 @@ class WalletDLCSetupTest extends BitcoinSDualWalletTest { accept.fundingInputs .map(_.output.value) .sum >= accept.collateral) - assert(accept.collateral == offer.contractInfo.max - offer.collateral) + assert( + accept.collateral == offer.contractInfo.totalCollateral - offer.collateral) assert(accept.changeAddress.value.nonEmpty) } @@ -783,7 +791,8 @@ class WalletDLCSetupTest extends BitcoinSDualWalletTest { accept <- walletB.acceptDLCOffer(offer, None, None, None) _ = { assert(accept.fundingInputs.nonEmpty) - assert(accept.collateral == offer.contractInfo.max - offer.collateral) + assert( + accept.collateral == offer.contractInfo.maxOffererPayout - offer.collateral) assert(accept.changeAddress.value.nonEmpty) } @@ -982,6 +991,30 @@ class WalletDLCSetupTest extends BitcoinSDualWalletTest { } yield res } + it must "fail to accept an offer when you do not have enough money in the wallet" in { + wallets => + val walletA = wallets._1.wallet + val walletB = wallets._2.wallet + + val offerData: DLCOffer = + DLCWalletUtil.buildDLCOffer(totalCollateral = Bitcoins(100)) + + for { + offer <- walletA.createDLCOffer( + contractInfo = offerData.contractInfo, + collateral = offerData.collateral, + feeRateOpt = Some(offerData.feeRate), + locktime = offerData.timeouts.contractMaturity.toUInt32, + refundLocktime = UInt32.max, + peerAddressOpt = None, + externalPayoutAddressOpt = None, + externalChangeAddressOpt = None + ) + _ <- recoverToSucceededIf[RuntimeException]( + walletB.acceptDLCOffer(offer, None, None, None)) + } yield succeed + } + it must "fail to create an offer with an invalid announcement signature" in { wallets => val walletA = wallets._1.wallet diff --git a/dlc-wallet/src/main/scala/org/bitcoins/dlc/wallet/DLCWallet.scala b/dlc-wallet/src/main/scala/org/bitcoins/dlc/wallet/DLCWallet.scala index d6269b65ea..233d73eff2 100644 --- a/dlc-wallet/src/main/scala/org/bitcoins/dlc/wallet/DLCWallet.scala +++ b/dlc-wallet/src/main/scala/org/bitcoins/dlc/wallet/DLCWallet.scala @@ -692,8 +692,7 @@ abstract class DLCWallet val dlcId = calcDLCId(offer.fundingInputs.map(_.outPoint)) - val collateral = offer.contractInfo.max - offer.collateral - + val collateral = offer.contractInfo.totalCollateral - offer.collateral logger.debug(s"Checking if Accept (${dlcId.hex}) has already been made") for { dlcAcceptOpt <- DLCAcceptUtil.findDLCAccept(dlcId = dlcId, diff --git a/docs/core/dlc.md b/docs/core/dlc.md index 2cdea0f80b..b915afb2a0 100644 --- a/docs/core/dlc.md +++ b/docs/core/dlc.md @@ -152,7 +152,7 @@ val oracleInfo = NumericMultiOracleInfo( ) val contractInfo = SingleContractInfo(totalCollateral, ContractOraclePair.NumericPair(descriptor, oracleInfo)) -contractInfo.max +contractInfo.totalCollateral contractInfo.allOutcomes.length val signingOracles = oracleInfo.singleOracleInfos.take(3) diff --git a/testkit/src/main/scala/org/bitcoins/testkit/wallet/DLCWalletUtil.scala b/testkit/src/main/scala/org/bitcoins/testkit/wallet/DLCWalletUtil.scala index fe9d756819..7d31a438d4 100644 --- a/testkit/src/main/scala/org/bitcoins/testkit/wallet/DLCWalletUtil.scala +++ b/testkit/src/main/scala/org/bitcoins/testkit/wallet/DLCWalletUtil.scala @@ -56,18 +56,35 @@ object DLCWalletUtil extends Logging { val total: Satoshis = (expectedDefaultAmt / Satoshis(2)).satoshis val half: Satoshis = (total / Satoshis(2)).satoshis - val sampleOutcomes: Vector[(EnumOutcome, Satoshis)] = Vector( - EnumOutcome(winStr) -> (expectedDefaultAmt / Satoshis(2)).satoshis, - EnumOutcome(loseStr) -> Satoshis.zero) + val sampleOutcomes: Vector[(EnumOutcome, Satoshis)] = + Vector(EnumOutcome(winStr) -> total, EnumOutcome(loseStr) -> Satoshis.zero) + + val nonWinnerTakeAllOutcomes: Vector[(EnumOutcome, Satoshis)] = { + val offset = Satoshis(10000) + Vector( + EnumOutcome(winStr) -> (total.satoshis - offset).satoshis, + EnumOutcome(loseStr) -> offset + ) + } lazy val sampleContractDescriptor: EnumContractDescriptor = EnumContractDescriptor(sampleOutcomes) + lazy val sampleContractDescriptorNonWinnerTakeAll: EnumContractDescriptor = { + EnumContractDescriptor(nonWinnerTakeAllOutcomes) + } + lazy val sampleOracleInfo: EnumSingleOracleInfo = EnumSingleOracleInfo.dummyForKeys(oraclePrivKey, rValue, sampleOutcomes.map(_._1)) + lazy val sampleOracleInfoNonWinnerTakeAll: EnumSingleOracleInfo = { + EnumSingleOracleInfo.dummyForKeys(oraclePrivKey, + rValue, + nonWinnerTakeAllOutcomes.map(_._1)) + } + lazy val invalidOracleInfo: EnumSingleOracleInfo = { val info = EnumSingleOracleInfo.dummyForKeys(oraclePrivKey, rValue, @@ -81,11 +98,21 @@ object DLCWalletUtil extends Logging { lazy val sampleContractOraclePair: ContractOraclePair.EnumPair = ContractOraclePair.EnumPair(sampleContractDescriptor, sampleOracleInfo) + lazy val sampleContractOraclePairNonWinnerTakeAll: ContractOraclePair.EnumPair = { + ContractOraclePair.EnumPair(sampleContractDescriptorNonWinnerTakeAll, + sampleOracleInfoNonWinnerTakeAll) + } + lazy val invalidContractOraclePair: ContractOraclePair.EnumPair = ContractOraclePair.EnumPair(sampleContractDescriptor, invalidOracleInfo) - lazy val sampleContractInfo: ContractInfo = - SingleContractInfo(half, sampleContractOraclePair) + lazy val sampleContractInfo: SingleContractInfo = + SingleContractInfo(totalCollateral = total, + contractOraclePair = sampleContractOraclePair) + + lazy val sampleContractInfoNonWinnerTakeAll: SingleContractInfo = { + SingleContractInfo(total, sampleContractOraclePairNonWinnerTakeAll) + } val amt2: Satoshis = Satoshis(100000) @@ -186,6 +213,14 @@ object DLCWalletUtil extends Logging { timeouts = dummyTimeouts ) + lazy val sampleDLCOfferNonWinnerTakeAll: DLCOffer = + sampleDLCOffer.copy(contractInfo = sampleContractInfoNonWinnerTakeAll) + + def buildDLCOffer(totalCollateral: CurrencyUnit): DLCOffer = { + val ci = sampleContractInfo.copy(totalCollateral = totalCollateral.satoshis) + sampleDLCOffer.copy(contractInfo = ci) + } + lazy val sampleDLCOffer2 = DLCOffer( protocolVersionOpt = DLCOfferTLV.currentVersionOpt, contractInfo = sampleContractInfo2,