[WIP] Ref script size fee calculation + Ref input support (tx builder) #349
Conversation
Fee calculation has changed to take into account ref script sizes via the new protocol parameter `min_fee_ref_script_cost_per_byte`.
| @@ -227,10 +228,54 @@ fn min_fee(tx_builder: &TransactionBuilder) -> Result<Coin, TxBuilderError> { | |||
| fn min_fee_with_exunits(tx_builder: &TransactionBuilder) -> Result<Coin, TxBuilderError> { | |||
There was a problem hiding this comment.
min_fee might possibly need updating I think, or even rethinking these two because we have more components to the fee calculations now - but maybe not since can you even possibly incur this ref script fee if you don't have exunits involved? but it could be involved just give temporarily incorrect values (e.g. for seeing how much doing something changes fee mid-build before plutus contract is added)
| pub fn min_ref_script_fee( | ||
| tx: &Transaction, | ||
| linear_fee: &LinearFee, | ||
| total_ref_script_size: u64, |
There was a problem hiding this comment.
It isn't possible to know the original sizes so we can't compute this from just the Transaction anymore
| // } | ||
| // } | ||
|
|
||
| let ref_script_orig_sizes: HashMap<ScriptHash, u64> = if let Some(ref_inputs) = &tx_builder.reference_inputs { |
There was a problem hiding this comment.
From reading CIP33 it says:
The key idea is to use reference inputs and modified outputs which carry actual scripts ("reference scripts"), and allow such reference scripts to satisfy the script witnessing requirement for a transaction. This means that the transaction which uses the script will not need to provide it at all, so long as it referenced an output which contained the script.
so I'm going on the assumption that if the hash matches something created as a reference script in a reference input's output that it must be a reference script and will incur this extra fee
other option: be explicit during tx builder/input builder and get the info from there
rooooooooob
changed the title
| // https://github.com/IntersectMBO/cardano-ledger/blob/7e65f0365eef647b9415e3fe9b3c35561761a3d5/eras/conway/impl/src/Cardano/Ledger/Conway/Tx.hs#L84 | ||
| // https://github.com/IntersectMBO/cardano-ledger/blob/a34f878c56763d138d2203d8ba84b3af64d94fce/eras/conway/impl/src/Cardano/Ledger/Conway/UTxO.hs#L152 | ||
| use num::{rational::Ratio, CheckedAdd, CheckedMul}; | ||
|
|
There was a problem hiding this comment.
we might need Ratio<BigNum> during calculations then conversion afterwards
There was a problem hiding this comment.
Done, but do we care about other usage of the fraction crate? I changed this here since it just panics even though the final operation could fall into a u64 fine if the intermediate denom/numerators don't. The other place we use the fraction crate is in genesis parsing.
rooooooooob
changed the title
| let mut total_ref_script_size = 0; | ||
| for utxo in tx_builder.inputs.iter() { | ||
| if let Some(Credential::Script { hash, .. }) = utxo.output.address().payment_cred() { | ||
| println!(" ------ searching: {}", hash.to_hex()); |
There was a problem hiding this comment.
Probably we want to remove all these print statements?
There was a problem hiding this comment.
Yes. Sorry, I forgot to set it back to draft. I just wanted to check mergability.
| @@ -402,6 +463,7 @@ pub struct TransactionBuilder { | |||
| utxos: Vec<InputBuilderResult>, | |||
| collateral_return: Option<TransactionOutput>, | |||
| reference_inputs: Option<Vec<TransactionUnspentOutput>>, | |||
| //reference_scripts_used: Vec<Script>, | |||
| PlutusScriptWitness::Ref(ref_script) => { | ||
| PlutusScriptWitness::Ref(ref_script_hash) => { | ||
| // it could also be a reference script - check those too | ||
| // TODO: do we want to change how we store ref scripts to cache the hash to avoid re-hashing (slow on wasm) every time an input is added? |
There was a problem hiding this comment.
TODO? Okay to merge with this?
There was a problem hiding this comment.
With this, yes. I don't think it's too bad of a performance hit for now (did not profile it - just going off past experience with hashing in the builder and we don't support asm.js in the new CML anyway so it's not as ridiculous a difference) and it's only when using ref scripts and we can always optimize later, using this comment as a reminder.
rooooooooob
added
bug
Fee calculation has changed to take into account ref script sizes via the new protocol parameter
min_fee_ref_script_cost_per_byte.TxBuilder has been updated to actually take into account reference inputs. Before it let you add them to transactions but the witness checks were oblivious to them.
Draft PR still. I have never worked with reference inputs or reference scripts before so some of this could be incorrect still