Emurgo / cardano-serialization-lib

This is a library, written in Rust, for serialization & deserialization of data structures used in Cardano's Haskell implementation of Alonzo along with useful utility functions.
Other
231 stars 125 forks source link

Add_inputs_from ignores the min_ada involved in the change output #616

Closed tqueri closed 1 week ago

tqueri commented 1 year ago

I've been working around that issue for a while right now but I think it's a good idea to share the issue. When using add_inputs_from with utxos containing nfts, the balance without the coin selection doesnt take into consideration the amount of lovelace that will be needed for the change output.

So let's say, you have 2 utxos containing each 1 nft. Both of them will have the minimum ada (around 1.2 ada each) so the total in your wallet at this point is 2.4 ada + 2 nfts, if you want to send 1 ada to someone (normally you should have enough) the add_inputs_from function will only add 1 of the 2 utxos considering 1.2 ada is enough to cover the 1 ada output + the tx fees. Which is false, in that instance, all utxos should be included in the transaction and the 2 nfts now merged will only need 1.4 min ada instead of the initial 1.2 each.

I made a simple transaction builder using CSL to show the issue.

import {
  TransactionBuilder,
  TransactionBuilderConfigBuilder,
  LinearFee,
  BigNum,
  TransactionUnspentOutputs,
  TransactionUnspentOutput,
  TransactionInput,
  TransactionHash,
  TransactionOutputBuilder,
  Address,
  MultiAsset,
  ScriptHash,
  Assets,
  AssetName,
} from "@emurgo/cardano-serialization-lib-nodejs";

const parameters = {
  epoch: 406,
  min_fee_a: 44,
  min_fee_b: 155381,
  max_block_size: 90112,
  max_tx_size: 16384,
  max_block_header_size: 1100,
  key_deposit: "2000000",
  pool_deposit: "500000000",
  e_max: 18,
  n_opt: 500,
  a0: 0.3,
  rho: 0.003,
  tau: 0.2,
  decentralisation_param: 0,
  extra_entropy: null,
  protocol_major_ver: 8,
  protocol_minor_ver: 0,
  min_utxo: "4310",
  min_pool_cost: "340000000",
  nonce: "64037a13fdb37620cfa5abb138a776263c51a28f13f9e676bcfd0e82c8043581",
  price_mem: 0.0577,
  price_step: 0.0000721,
  max_tx_ex_mem: "14000000",
  max_tx_ex_steps: "10000000000",
  max_block_ex_mem: "62000000",
  max_block_ex_steps: "20000000000",
  max_val_size: "5000",
  collateral_percent: 150,
  max_collateral_inputs: 3,
  coins_per_utxo_size: "4310",
  coins_per_utxo_word: "4310",
};
const fromHex = (
  hex:
    | WithImplicitCoercion<string>
    | { [Symbol.toPrimitive](hint: "string"): string }
) => Buffer.from(hex, "hex");
test("Add Inputs From, all utxos", async () => {
  const {
    coins_per_utxo_size,
    min_fee_a,
    min_fee_b,
    max_val_size,
    max_tx_size,
    pool_deposit,
    key_deposit,
  } = parameters;
  const testAddress =
    "addr1q8seyqha6kdmv9l8xxneek9zahghsmksnxu6lrmwwzh9dg8zrhhjr476dsq2fgmety6j3adv9t3wcycv0jp4ajr3z8tqvnajjn";
  const txBuilder = TransactionBuilder.new(
    TransactionBuilderConfigBuilder.new()
      .fee_algo(
        LinearFee.new(
          BigNum.from_str(String(min_fee_a)),
          BigNum.from_str(String(min_fee_b))
        )
      )
      .coins_per_utxo_byte(BigNum.from_str(String(coins_per_utxo_size)))
      .pool_deposit(BigNum.from_str(String(pool_deposit)))
      .key_deposit(BigNum.from_str(String(key_deposit)))
      .max_value_size(Number(max_val_size))
      .max_tx_size(max_tx_size)
      .build()
  );

  const utxos = TransactionUnspentOutputs.new();
  const multiAsset1 = MultiAsset.new();
  const assets1 = Assets.new();
  assets1.insert(AssetName.new(fromHex("test")), BigNum.one());
  multiAsset1.insert(
    ScriptHash.from_hex(
      "00000000000000000000000000000000000000000000000000000000"
    ),
    assets1
  );
  utxos.add(
    TransactionUnspentOutput.new(
      TransactionInput.new(
        TransactionHash.from_hex(
          "0000000000000000000000000000000000000000000000000000000000000000"
        ),
        0
      ),
      TransactionOutputBuilder.new()
        .with_address(Address.from_bech32(testAddress))
        .next()
        .with_coin_and_asset(BigNum.from_str(String(2000000)), multiAsset1)
        .build()
    )
  );

  const multiAsset2 = MultiAsset.new();
  const assets2 = Assets.new();
  assets2.insert(AssetName.new(fromHex("test2")), BigNum.one());
  multiAsset2.insert(
    ScriptHash.from_hex(
      "00000000000000000000000000000000000000000000000000000000"
    ),
    assets1
  );
  utxos.add(
    TransactionUnspentOutput.new(
      TransactionInput.new(
        TransactionHash.from_hex(
          "0000000000000000000000000000000000000000000000000000000000000000"
        ),
        1
      ),
      TransactionOutputBuilder.new()
        .with_address(Address.from_bech32(testAddress))
        .next()
        .with_coin_and_asset(BigNum.from_str(String(2000000)), multiAsset2)
        .build()
    )
  );

  txBuilder.add_output(
    TransactionOutputBuilder.new()
      .with_address(Address.from_bech32(testAddress))
      .next()
      .with_coin(BigNum.from_str("1000000"))
      .build()
  );

  for (let i = 0; i < utxos.len(); i++) {
    const utxo = utxos.get(i);
    txBuilder.add_input(
      utxo.output().address(),
      TransactionInput.new(utxo.input().transaction_id(), utxo.input().index()),
      utxo.output().amount()
    );
  }

  txBuilder.add_change_if_needed(Address.from_bech32(testAddress));

  const tx = txBuilder.build_tx();
  expect(tx.to_hex()).toBe(
    "84a30082825820000000000000000000000000000000000000000000000000000000000000000000825820000000000000000000000000000000000000000000000000000000000000000001018282583901e19202fdd59bb617e731a79cd8a2edd1786ed099b9af8f6e70ae56a0e21def21d7da6c00a4a379593528f5ac2ae2ec130c7c835ec87111d61a000f424082583901e19202fdd59bb617e731a79cd8a2edd1786ed099b9af8f6e70ae56a0e21def21d7da6c00a4a379593528f5ac2ae2ec130c7c835ec87111d6821a002b2a17a1581c00000000000000000000000000000000000000000000000000000000a14002021a00029ca9a0f5f6"
  );
  console.log(tx);
});

test("Add Inputs From", async () => {
  const {
    coins_per_utxo_size,
    min_fee_a,
    min_fee_b,
    max_val_size,
    max_tx_size,
    pool_deposit,
    key_deposit,
  } = parameters;
  const testAddress =
    "addr1q8seyqha6kdmv9l8xxneek9zahghsmksnxu6lrmwwzh9dg8zrhhjr476dsq2fgmety6j3adv9t3wcycv0jp4ajr3z8tqvnajjn";
  const txBuilder = TransactionBuilder.new(
    TransactionBuilderConfigBuilder.new()
      .fee_algo(
        LinearFee.new(
          BigNum.from_str(String(min_fee_a)),
          BigNum.from_str(String(min_fee_b))
        )
      )
      .coins_per_utxo_byte(BigNum.from_str(String(coins_per_utxo_size)))
      .pool_deposit(BigNum.from_str(String(pool_deposit)))
      .key_deposit(BigNum.from_str(String(key_deposit)))
      .max_value_size(Number(max_val_size))
      .max_tx_size(max_tx_size)
      .build()
  );

  const utxos = TransactionUnspentOutputs.new();
  const multiAsset1 = MultiAsset.new();
  const assets1 = Assets.new();
  assets1.insert(AssetName.new(fromHex("test")), BigNum.one());
  multiAsset1.insert(
    ScriptHash.from_hex(
      "00000000000000000000000000000000000000000000000000000000"
    ),
    assets1
  );
  utxos.add(
    TransactionUnspentOutput.new(
      TransactionInput.new(
        TransactionHash.from_hex(
          "0000000000000000000000000000000000000000000000000000000000000000"
        ),
        0
      ),
      TransactionOutputBuilder.new()
        .with_address(Address.from_bech32(testAddress))
        .next()
        .with_coin_and_asset(BigNum.from_str(String(2000000)), multiAsset1)
        .build()
    )
  );

  const multiAsset2 = MultiAsset.new();
  const assets2 = Assets.new();
  assets2.insert(AssetName.new(fromHex("test2")), BigNum.one());
  multiAsset2.insert(
    ScriptHash.from_hex(
      "00000000000000000000000000000000000000000000000000000000"
    ),
    assets1
  );
  utxos.add(
    TransactionUnspentOutput.new(
      TransactionInput.new(
        TransactionHash.from_hex(
          "0000000000000000000000000000000000000000000000000000000000000000"
        ),
        1
      ),
      TransactionOutputBuilder.new()
        .with_address(Address.from_bech32(testAddress))
        .next()
        .with_coin_and_asset(BigNum.from_str(String(2000000)), multiAsset2)
        .build()
    )
  );

  txBuilder.add_output(
    TransactionOutputBuilder.new()
      .with_address(Address.from_bech32(testAddress))
      .next()
      .with_coin(BigNum.from_str("1000000"))
      .build()
  );

  txBuilder.add_inputs_from(utxos, 2);

  txBuilder.add_change_if_needed(Address.from_bech32(testAddress));

  const tx = txBuilder.build_tx();
  console.log(tx);
});

From my understanding, all the issue is caused by add_inputs_from since when handling manually the coin selection, its possible to build the transaction with the same utxos. Thanks

lisicky commented 1 year ago

Hi @tqueri ! Unfortunately current coin selection implementation does't know about the change. https://github.com/Emurgo/cardano-serialization-lib/issues/553#issuecomment-1326260015 And unfortunately we don't have plans to fix in a short time. But it is an open source project, feel free to contribute.

lisicky commented 1 week ago

Fixed by add_inputs_from_and_change function in the CSL 12.0.0, please use add_inputs_from_and_change instead add_inputs_from. Thanks @twwu123 for your implementation.