Repository navigation
Fix reference script fee calculation #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
eb45c27
78b00be
231539e
7b3c800
7950a78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ use crate::assets::MultiAsset; | |
| use crate::assets::{AssetArithmeticError, Mint}; | ||
| use crate::auxdata::AuxiliaryData; | ||
| use crate::builders::output_builder::TransactionOutputBuilder; | ||
| use crate::certs::{Certificate, Credential}; | ||
| use crate::certs::Certificate; | ||
| use crate::crypto::hash::{calc_script_data_hash, hash_auxiliary_data, ScriptDataHashError}; | ||
| use crate::crypto::{BootstrapWitness, Vkeywitness}; | ||
| use crate::deposit::{internal_get_deposit, internal_get_implicit_input}; | ||
|
|
@@ -31,12 +31,14 @@ use crate::transaction::{ | |
| DatumOption, ScriptRef, Transaction, TransactionBody, TransactionInput, TransactionOutput, | ||
| TransactionWitnessSet, | ||
| }; | ||
| use crate::{assets::AssetName, Coin, ExUnitPrices, NetworkId, PolicyId, Value, Withdrawals}; | ||
| use crate::{ | ||
| assets::AssetName, Coin, ExUnitPrices, NetworkId, PolicyId, Script, Value, Withdrawals, | ||
| }; | ||
| use cbor_event::{de::Deserializer, se::Serializer}; | ||
| use cml_core::ordered_hash_map::OrderedHashMap; | ||
| use cml_core::serialization::{CBORReadLen, Deserialize}; | ||
| use cml_core::{ArithmeticError, DeserializeError, DeserializeFailure, Slot}; | ||
| use cml_crypto::{Ed25519KeyHash, ScriptDataHash, ScriptHash, Serialize}; | ||
| use cml_crypto::{Ed25519KeyHash, RawBytesEncoding, ScriptDataHash, ScriptHash, Serialize}; | ||
| use num::Zero; | ||
| use rand::Rng; | ||
| use std::collections::{BTreeSet, HashMap}; | ||
|
|
@@ -224,41 +226,42 @@ fn min_fee(tx_builder: &TransactionBuilder) -> Result<Coin, TxBuilderError> { | |
| crate::fees::min_no_script_fee(&full_tx, &tx_builder.config.fee_algo).map_err(Into::into) | ||
| } | ||
|
|
||
| fn min_fee_with_exunits(tx_builder: &TransactionBuilder) -> Result<Coin, TxBuilderError> { | ||
| let full_tx = fake_full_tx(tx_builder, tx_builder.build_body()?)?; | ||
| // we can't know the of scripts yet as they can't be calculated until we build the tx | ||
|
|
||
| fn ref_script_orig_size_builder(utxo: &TransactionUnspentOutput) -> Option<(ScriptHash, u64)> { | ||
| utxo.output.script_ref().map(|script_ref| { | ||
| ( | ||
| script_ref.hash(), | ||
| script_ref | ||
| .raw_plutus_bytes() | ||
| .expect("TODO: handle this") | ||
| .len() as u64, | ||
| ) | ||
| }) | ||
| fn ref_script_orig_size(script_ref: &ScriptRef) -> u64 { | ||
| match script_ref { | ||
| Script::Native { script, .. } => script.to_cbor_bytes().len() as u64, | ||
| Script::PlutusV1 { script, .. } => script.to_raw_bytes().len() as u64, | ||
| Script::PlutusV2 { script, .. } => script.to_raw_bytes().len() as u64, | ||
| Script::PlutusV3 { script, .. } => script.to_raw_bytes().len() as u64, | ||
| } | ||
| } | ||
|
|
||
| let ref_script_orig_sizes: HashMap<ScriptHash, u64> = | ||
| if let Some(ref_inputs) = &tx_builder.reference_inputs { | ||
| ref_inputs | ||
| .iter() | ||
| .filter_map(ref_script_orig_size_builder) | ||
| .collect() | ||
| } else { | ||
| HashMap::default() | ||
| }; | ||
| fn total_ref_script_size_for_fee(tx_builder: &TransactionBuilder) -> Result<u64, TxBuilderError> { | ||
| let mut ref_script_inputs = BTreeSet::new(); | ||
| let mut total_ref_script_size = 0u64; | ||
|
|
||
| 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() { | ||
| if let Some(orig_size) = ref_script_orig_sizes.get(hash) { | ||
| total_ref_script_size += *orig_size; | ||
| for utxo in tx_builder | ||
| .inputs | ||
| .iter() | ||
| .chain(tx_builder.reference_inputs.iter().flatten()) | ||
| { | ||
| if ref_script_inputs.insert(utxo.input.clone()) { | ||
| if let Some(script_ref) = utxo.output.script_ref() { | ||
| total_ref_script_size = total_ref_script_size | ||
| .checked_add(ref_script_orig_size(script_ref)) | ||
| .ok_or(ArithmeticError::IntegerOverflow)?; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(total_ref_script_size) | ||
| } | ||
|
|
||
| fn min_fee_with_exunits(tx_builder: &TransactionBuilder) -> Result<Coin, TxBuilderError> { | ||
| let full_tx = fake_full_tx(tx_builder, tx_builder.build_body()?)?; | ||
| // we can't know the of scripts yet as they can't be calculated until we build the tx | ||
|
|
||
| let total_ref_script_size = total_ref_script_size_for_fee(tx_builder)?; | ||
|
|
||
| crate::fees::min_fee( | ||
| &full_tx, | ||
| &tx_builder.config.fee_algo, | ||
|
|
@@ -1718,7 +1721,7 @@ pub fn add_change_if_needed( | |
| let fee = match &builder.fee { | ||
| None => builder.min_fee(include_exunits), | ||
| // generating the change output involves changing the fee | ||
| Some(_x) => return Ok(false), | ||
| Some(set_fee) => Ok(set_fee.clone()), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this now contradicts the comment directly above it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment was already contradicted, generating the change output did not change the fee if there was a fee set. |
||
| }?; | ||
|
|
||
| // note: can't add datum / script_ref to change | ||
|
|
@@ -2250,6 +2253,56 @@ mod tests { | |
| ((spend, spend_cred), (stake, stake_cred), address) | ||
| } | ||
|
|
||
| fn output_with_ref_script(address: &Address, script_ref: ScriptRef) -> TransactionOutput { | ||
| TransactionOutput::new( | ||
| address.clone(), | ||
| Value::from(5_000_000), | ||
| None, | ||
| Some(script_ref), | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| fn ref_script_fee_size_counts_regular_and_reference_inputs() { | ||
| let mut tx_builder = create_default_tx_builder(); | ||
| let (_, _, address) = create_account(); | ||
| let script = Script::new_plutus_v2(PlutusV2Script::new(vec![1, 2, 3, 4])); | ||
| let other_script = Script::new_plutus_v1(PlutusV1Script::new(vec![5, 6, 7])); | ||
|
|
||
| let spent_ref_script_utxo = TransactionUnspentOutput::new( | ||
| TransactionInput::new(genesis_id(), 0), | ||
| output_with_ref_script(&address, script.clone()), | ||
| ); | ||
| let repeated_script_ref_input = TransactionUnspentOutput::new( | ||
| TransactionInput::new(genesis_id(), 1), | ||
| output_with_ref_script(&address, script), | ||
| ); | ||
| let other_ref_input = TransactionUnspentOutput::new( | ||
| TransactionInput::new(genesis_id(), 2), | ||
| output_with_ref_script(&address, other_script), | ||
| ); | ||
|
|
||
| tx_builder.inputs.push(spent_ref_script_utxo.clone()); | ||
| tx_builder.reference_inputs = Some(vec![ | ||
| spent_ref_script_utxo, | ||
| repeated_script_ref_input, | ||
| other_ref_input, | ||
| ]); | ||
|
|
||
| assert_eq!(total_ref_script_size_for_fee(&tx_builder).unwrap(), 11); | ||
| } | ||
|
|
||
| #[test] | ||
| fn native_ref_script_size_does_not_panic() { | ||
| let script_ref = Script::new_native(NativeScript::new_script_pubkey(fake_key_hash(1))); | ||
| let expected_size = match &script_ref { | ||
| Script::Native { script, .. } => script.to_cbor_bytes().len() as u64, | ||
| _ => unreachable!(), | ||
| }; | ||
|
|
||
| assert_eq!(ref_script_orig_size(&script_ref), expected_size); | ||
| } | ||
|
|
||
| #[test] | ||
| fn build_tx_with_change() { | ||
| let mut tx_builder = create_default_tx_builder(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1025,7 +1025,28 @@ impl_wasm_list!( | |
| StakeCredentialList | ||
| ); | ||
|
|
||
| pub type SubCoin = Rational; | ||
| #[derive(Clone, Debug)] | ||
| #[wasm_bindgen] | ||
| pub struct SubCoin(cml_chain::SubCoin); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this needed for anything? |
||
|
|
||
| impl_wasm_cbor_json_api!(SubCoin); | ||
|
|
||
| impl_wasm_conversions!(cml_chain::SubCoin, SubCoin); | ||
|
|
||
| #[wasm_bindgen] | ||
| impl SubCoin { | ||
| pub fn numerator(&self) -> u64 { | ||
| self.0.numerator | ||
| } | ||
|
|
||
| pub fn denominator(&self) -> u64 { | ||
| self.0.denominator | ||
| } | ||
|
|
||
| pub fn new(numerator: u64, denominator: u64) -> Self { | ||
| Self(cml_chain::SubCoin::new(numerator, denominator)) | ||
| } | ||
| } | ||
|
|
||
| impl_wasm_list!( | ||
| cml_chain::transaction::TransactionBody, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ const pathToRepo = path.join(__dirname, '..', crateName, 'wasm'); | |
| const oldPkg = require(`${pathToRepo}/publish/package.json`); | ||
|
|
||
| const packageNameRoot = hyphenRepoName.split("-wasm")[0]; | ||
| oldPkg.name = '@dcspark/' + packageNameRoot + buildType; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you'll need to revert this one |
||
| oldPkg.name = '@anastasia-labs/' + packageNameRoot + buildType; | ||
| if (buildType === '-browser' || buildType === '-asmjs') { | ||
| // due to a bug in wasm-pack, this file is missing from browser builds | ||
| const missingFile = `${underscoreRepoName}_bg.js`; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO is what was already there prior to any of my changes.