Fixed regarding UTXO selection and fee calculation

- Fix fee calculation for P2PKH inputs
- Do not add zero-value fungible tokens when selecting UTXOs
- Do not add token UTXOs needlessly when amountNeeded === 0

Author: rkalis

examples/common.ts

@@ -19,15 +19,15 @@ if (typeof aliceNode === 'string') throw new Error();
 export const alicePub = secp256k1.derivePublicKeyCompressed(aliceNode.privateKey) as Uint8Array;
 export const alicePriv = aliceNode.privateKey;
 export const alicePkh = hash160(alicePub);
-export const aliceAddress = encodeCashAddress('bchtest', 'p2pkh', alicePkh);
+export const aliceAddress = encodeCashAddress('bchtest', 'p2pkhWithTokens', alicePkh);
 
 // Derive Bob's private key, public key, public key hash and address
 const bobNode = deriveHdPath(rootNode, `${baseDerivationPath}/1`);
 if (typeof bobNode === 'string') throw new Error();
 export const bobPub = secp256k1.derivePublicKeyCompressed(bobNode.privateKey) as Uint8Array;
 export const bobPriv = bobNode.privateKey;
 export const bobPkh = hash160(bobPub);
-export const bobAddress = encodeCashAddress('bchtest', 'p2pkh', bobPkh);
+export const bobAddress = encodeCashAddress('bchtest', 'p2pkhWithTokens', bobPkh);
 
 // Initialise a price oracle with a private key
 const oracleNode = deriveHdPath(rootNode, `${baseDerivationPath}/2`);
@@ -36,4 +36,4 @@ export const oraclePub = secp256k1.derivePublicKeyCompressed(oracleNode.privateK
 export const oraclePriv = oracleNode.privateKey;
 export const oracle = new PriceOracle(oracleNode.privateKey);
 export const oraclePkh = hash160(oraclePub);
-export const oracleAddress = encodeCashAddress('bchtest', 'p2pkh', oraclePkh);
+export const oracleAddress = encodeCashAddress('bchtest', 'p2pkhWithTokens', oraclePkh);

examples/pat_bug.ts

@@ -0,0 +1,110 @@
+import { stringify } from '@bitauth/libauth';
+import { compileString } from 'cashc';
+import {
+  Contract,
+  ElectrumNetworkProvider,
+  Network,
+  SignatureTemplate,
+} from 'cashscript';
+import { bobAddress, bobPriv } from './common.js';
+
+const createContract = (network: Network = Network.REGTEST): Contract => {
+  const script = `
+  contract test(int nonce) {
+    function test() {
+      require(nonce == nonce);
+      int minerFee = 800;
+      int tokenValue = 1000;
+      int sentValue = tx.inputs[1].value;
+      int changeValue = sentValue - minerFee - tokenValue;
+
+      require(tx.inputs.length == 2);
+
+      // handle change
+      if (changeValue < 546) {
+        // discard dust change, whatever dust goes to miner fee
+        // so in this case total fee = 800 + dust
+        require(tx.outputs.length == 2);
+      } else {
+        // allow change output as outputs[2]
+        require(tx.outputs.length == 3);
+        // this forces the fee to be exactly 800
+        require(tx.outputs[2].value == changeValue);
+        // Require that the change output does not mint any NFTs
+        require(tx.outputs[2].tokenCategory == 0x);
+        // have the change address be the same as funding address
+        bytes changeBytecode = tx.inputs[1].lockingBytecode;
+        require(tx.outputs[2].lockingBytecode == changeBytecode);
+      }
+    }
+  }
+  `;
+
+  const artifact = compileString(script);
+  const nonce = 123456n;
+
+  return new Contract(
+    artifact,
+    [nonce],
+    { provider: new ElectrumNetworkProvider(network) },
+  );
+};
+
+const minerFee = 800n;
+const tokenValue = 1000n;
+
+const contract = createContract(Network.CHIPNET);
+
+console.log('contract address', contract.tokenAddress);
+console.log('bob address', bobAddress);
+
+const tokenCategory = 'dc92b5d83b2d2fc0b20a135664367099a0d87dcdd5e1b40504b583fdc445839b';
+
+const contractUtxos = await contract.getUtxos();
+const mintingUtxo = contractUtxos.find((utxo) => utxo.token?.category === tokenCategory);
+
+const bobUtxos = await new ElectrumNetworkProvider(Network.CHIPNET).getUtxos(bobAddress);
+const bobUtxo = bobUtxos.find((utxo) => utxo.satoshis >= 10000n);
+// console.log('bob', bobUtxo)
+
+if (!mintingUtxo || !bobUtxo) process.exit(1);
+
+const tx = await contract.functions.test()
+  .from(mintingUtxo)
+  .fromP2PKH(bobUtxo, new SignatureTemplate(bobPriv))
+  .to([
+    {
+      to: contract.tokenAddress,
+      amount: mintingUtxo.satoshis,
+      token: {
+        category: tokenCategory,
+        amount: 0n,
+        nft: {
+          capability: 'minting',
+          commitment: 'beef',
+        },
+      },
+    },
+    {
+      to: bobAddress,
+      amount: tokenValue,
+      token: {
+        category: tokenCategory,
+        amount: 0n,
+        nft: {
+          capability: 'none',
+          commitment: 'deadbeef',
+        },
+      },
+    },
+    {
+      to: bobAddress,
+      amount: bobUtxo.satoshis - tokenValue - minerFee,
+    },
+  ])
+  .withoutChange()
+  // .withoutTokenChange()
+  // .withHardcodedFee(BigInt(minerFee))
+  .send();
+
+console.log(stringify(tx));

packages/cashscript/src/Transaction.ts

@@ -42,6 +42,7 @@ import {
 } from './utils.js';
 import NetworkProvider from './network/NetworkProvider.js';
 import SignatureTemplate from './SignatureTemplate.js';
+import { P2PKH_INPUT_SIZE } from './constants.js';
 
 const bip68 = await import('bip68');
 
@@ -398,7 +399,7 @@ export class Transaction {
     );
 
     // Add one extra byte per input to over-estimate tx-in count
-    const inputSize = getInputSize(placeholderScript) + 1;
+    const contractInputSize = getInputSize(placeholderScript) + 1;
 
     // Note that we use the addPrecision function to add "decimal points" to BigInt numbers
 
@@ -410,7 +411,14 @@ export class Transaction {
     let satsAvailable = 0n;
     if (this.inputs.length > 0) {
       // If inputs are already defined, the user provided the UTXOs and we perform no further UTXO selection
-      if (!this.hardcodedFee) fee += addPrecision(this.inputs.length * inputSize * this.feePerByte);
+      if (!this.hardcodedFee) {
+        const totalInputSize = this.inputs.reduce(
+          (acc, input) => acc + (isSignableUtxo(input) ? P2PKH_INPUT_SIZE : contractInputSize),
+          0,
+        );
+        fee += addPrecision(totalInputSize * this.feePerByte);
+      }
+
       satsAvailable = addPrecision(this.inputs.reduce((acc, input) => acc + input.satoshis, 0n));
     } else {
       // If inputs are not defined yet, we retrieve the contract's UTXOs and perform selection
@@ -424,14 +432,14 @@ export class Transaction {
       for (const utxo of automaticTokenInputs) {
         this.inputs.push(utxo);
         satsAvailable += addPrecision(utxo.satoshis);
-        if (!this.hardcodedFee) fee += addPrecision(inputSize * this.feePerByte);
+        if (!this.hardcodedFee) fee += addPrecision(contractInputSize * this.feePerByte);
       }
 
       for (const utxo of bchUtxos) {
         if (satsAvailable > amount + fee) break;
         this.inputs.push(utxo);
         satsAvailable += addPrecision(utxo.satoshis);
-        if (!this.hardcodedFee) fee += addPrecision(inputSize * this.feePerByte);
+        if (!this.hardcodedFee) fee += addPrecision(contractInputSize * this.feePerByte);
       }
     }
 
@@ -480,12 +488,9 @@ const calculateTotalTokenAmount = (outputs: Array<Output | Utxo>, tokenCategory:
 
 const selectTokenUtxos = (utxos: Utxo[], amountNeeded: bigint, tokenCategory: string): Utxo[] => {
   const genesisUtxo = getTokenGenesisUtxo(utxos, tokenCategory);
+  if (genesisUtxo) return [genesisUtxo];
 
-  if (genesisUtxo) {
-    return [genesisUtxo];
-  }
-
-  const tokenUtxos = utxos.filter((utxo) => utxo.token?.category === tokenCategory);
+  const tokenUtxos = utxos.filter((utxo) => utxo.token?.category === tokenCategory && utxo.token?.amount > 0n);
 
   // We sort the UTXOs mainly so there is consistent behaviour between network providers
   // even if they report UTXOs in a different order
@@ -496,9 +501,9 @@ const selectTokenUtxos = (utxos: Utxo[], amountNeeded: bigint, tokenCategory: st
 
   // Add token UTXOs until we have enough to cover the amount needed (no fee calculation because it's a token)
   for (const utxo of tokenUtxos) {
+    if (amountAvailable >= amountNeeded) break;
     selectedUtxos.push(utxo);
     amountAvailable += utxo.token!.amount;
-    if (amountAvailable >= amountNeeded) break;
   }
 
   if (amountAvailable < amountNeeded) {

packages/cashscript/src/constants.ts

@@ -1,2 +1,3 @@
 export const VERSION_SIZE = 4;
 export const LOCKTIME_SIZE = 4;
+export const P2PKH_INPUT_SIZE = 32 + 4 + 1 + 1 + 65 + 1 + 33 + 4;

yarn.lock

@@ -2263,9 +2263,9 @@
   integrity sha512-Exw4yUWMBXM3X+8oqzJNRqZSwUAaS4+7NdvHqQuFi/d+synz++xmX3QIf+BFqneW8N31R8Ky+sikfZUXq07ggQ==
 
 "@types/node@^12.7.8":
-  version "12.12.54"
-  resolved "https://registry.yarnpkg.com/@types/node/-/node-12.12.54.tgz#a4b58d8df3a4677b6c08bfbc94b7ad7a7a5f82d1"
-  integrity sha512-ge4xZ3vSBornVYlDnk7yZ0gK6ChHf/CHB7Gl1I0Jhah8DDnEQqBzgohYG4FX4p81TNirSETOiSyn+y1r9/IR6w==
+  version "12.20.55"
+  resolved "https://registry.yarnpkg.com/@types/node/-/node-12.20.55.tgz#c329cbd434c42164f846b909bd6f85b5537f6240"
+  integrity sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==
 
 "@types/node@^14.14.28":
   version "14.14.28"