Fee calculation error when spending UTXO with scriptRef

[SynthLuvr]

Steps to reproduce:

• Create a wallet
• Attach a script to a UTXO
• Spend the UTXO

This will cause the fee calculation to be incorrect and cannot be submitted on-chain

[SynthLuvr]

Example on preview network:

const address =
  "addr_test1qqja0lh0lnpwd7hewzptwpay09mgqm2k0srqsrfp89nya2h3uq0x2xemxfac2lf7axszqezaw2tpahj2d23guffx687sqz8edy";
const utxos = await lucid.utxosAt(address);
const destination = await getRandomAddress(lucid);

lucid.selectWallet.fromAddress(address, utxos);
const tx = await lucid
  .newTx()
  .pay.ToAddress(destination, {
    "20a814aa7a68d16b8d236f21a4dcb1ed1e1b359faff67287da2c33dd626561636f6e":
      1n,
  })
  .complete();

[nikhils9]

I think I know what's being missed here. I'll get it fixed.

[nikhils9]

Currently, fee calculation is incorrect for a tx which spends utxos (containing ref scripts) added by lucid's coin section. After giving some though, I believe utxos with scriptRef attached to them should not be selected by coin selection in the first place. They should be spent if explicitly intended by the user using collectFrom. If we go with this approach, the fix to this issue would be very simple otherwise it would require more complex logic at the level of coin selection and fees calculation.

I would like to know your opinions. @solidsnakedev @SynthLuvr @keyan-m @hadelive

PS: A similar argument can be made about utxos with datums attached to them. But since such utxos would most likely be return outputs from dapps, I believe its better to include them in coin selection (as is the case currently).

[SynthLuvr]

Sometimes we filter out UTXOs with datums from the wallet ourselves to avoid them being spent. I think by default they should be included because there are dapps that attach datums as a "receipt."

Regarding scriptRef, I think it'd be fine to spend it explicitly via collectFrom.

[SynthLuvr]

When using collectFrom it's still the same error

[sourabhxyz]

This could be due to updated fee structure in Conway (active in preview testnet) where reference scripts now carry additional weight, more details here.

[SynthLuvr]

This could be due to updated fee structure in Conway (active in preview testnet) where reference scripts now carry additional weight, more details here.

Yes this is true. The fee for reading from reference scripts was already implemented in #195. However, this new issue is related to spending UTXOs with reference scripts attached. I don't think there should be a fee attached for spending reference scripts. However, assuming this stays in cardano-ledger, lucid-evolution will need to be updated to account for this extra fee.

[sourabhxyz]

@SynthLuvr thanks for explaining! I haven't looked carefully at this issue, but

However, this new issue is related to spending UTXOs with reference scripts attached.

In my understanding, this additional fee is enforced irrespective of whether you are actually making use of reference script in your tx or not. As long as spend inputs or reference inputs have script attached to them, they would contribute to fees.

[SynthLuvr]

In my understanding, this additional fee is enforced irrespective of whether you are actually making use of reference script in your tx or not. As long as spend inputs or reference inputs have script attached to them, they would contribute to fees.

Yes that's correct. I think this was overlooked in the cardano-ledger implementation. Given that this is related to a vulnerability that was exploited, the implementation may have been rushed. It doesn't make sense that this should be contributing to fees, because there's no vulnerability associated with spending data, only referencing data within validation logic.

Having said that, these are the current ledger rules for conway, so if these rules remain as-is, then we have to adapt and add support for the fee.

[sourabhxyz]

@SynthLuvr Not sure if I am following properly, but generating script context requires resolving inputs, in particular also mentioning associated script hash (if any) which thus a Plutus script can depend upon, so even if a given input's script is not being exercised for validation, it could be referred by other script.

[nikhils9]

so even if a given input's script is not being exercised for validation, it could be referred by other script.

That's an interesting point. I guess even if such a scriptRef is being referred by another script (which is witnessing the transaction), the witnessing script will bear the cost of deserializing the scriptRef thereby accounting for the cost via its increased exUnits. The ledger need not presume and charge every scriptRef in inputs being spent.

[nikhils9]

When using collectFrom it's still the same error

After spending some good amount of time, I was finally able to get this working. Will need to test it further though.

To get ref scripts fee working, without it being calculated by CML, I had to resort setting fees in CML using TransactionBuilder::set_fee which brings along its own set of issues. This issue was one such instance of it where the ref fees were being calculated correctly however the estimated fees were set before the coin selection was completed which left room for fees to increase further.

[SynthLuvr]

That's an interesting point. I guess even if such a scriptRef is being referred by another script (which is witnessing the transaction), the witnessing script will bear the cost of deserializing the scriptRef thereby accounting for the cost via its increased exUnits. The ledger need not presume and charge every scriptRef in inputs being spent.

Right. So effectively with the current ledger-rules, we're paying extra for no reason. If a reference is being used, it'll be witnessed, and thus we'll pay the fee. But I don't see the logic for how it's currently implemented where in essence scriptRef is no different than a datum.

[solidsnakedev]

just to clarify the deserialization of the reference scripts (flat encoded) are done by the cardano nodes, and according to the ledger it is mandatory to count all the inputs with reference scripts, even if they're two or more inputs with the same reference script.
Also I think we should do this calculation inside the recursive coin selection, and add the extra cost for each added input

lucid-evolution/packages/lucid/src/tx-builder/internal/CompleteTxBuilder.ts

Lines 636 to 692 in 2617572

	export const recursive = (
	inputs: UTxO[],
	requiredAssets: Assets,
	coinsPerUtxoByte: bigint,
	externalAssets: Assets = {},
	error?: TxBuilderError,
	): Effect.Effect<UTxO[], TxBuilderError> =>
	Effect.gen(function* () {
	let selected: UTxO[] = [];
	error ??= completeTxError(
	`Your wallet does not have enough funds to cover the required assets: ${stringify(requiredAssets)}`,
	);
	if (!Record.isEmptyRecord(requiredAssets)) {
	selected = selectUTxOs(inputs, requiredAssets);
	if (_Array.isEmptyArray(selected)) yield* error;
	}
	const selectedAssets: Assets = sumAssetsFromInputs(selected);
	let availableAssets: Assets = pipe(
	selectedAssets,
	Record.union(requiredAssets, (self, that) => self - that),
	Record.union(externalAssets, _BigInt.sum),
	);
	let extraLovelace: Assets | undefined = pipe(
	calculateExtraLovelace(availableAssets, coinsPerUtxoByte),
	Option.getOrUndefined,
	);
	let remainingInputs = inputs;
	while (extraLovelace) {
	remainingInputs = _Array.differenceWith(isEqualUTxO)(
	remainingInputs,
	selected,
	);
	const extraSelected = selectUTxOs(remainingInputs, extraLovelace);
	if (_Array.isEmptyArray(extraSelected)) {
	yield* completeTxError(
	`Your wallet does not have enough funds to cover required minimum ADA for change output: ${stringify(extraLovelace)}`,
	);
	}
	const extraSelectedAssets: Assets = sumAssetsFromInputs(extraSelected);
	selected = [...selected, ...extraSelected];
	availableAssets = Record.union(
	availableAssets,
	extraSelectedAssets,
	_BigInt.sum,
	);
	extraLovelace = pipe(
	calculateExtraLovelace(availableAssets, coinsPerUtxoByte),
	Option.getOrUndefined,
	);
	}
	return selected;
	});

[solidsnakedev]

There's also a CML update in the work, but I think we're much closer to get it fixed.
dcSpark/cardano-multiplatform-lib#349