Skip to content
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

feat(dart/catalyst_cardano_serialization)!: Implement tiered fee calculation for reference scripts #871

Merged
merged 9 commits into from
Sep 24, 2024

Conversation

ilap
Copy link
Contributor

@ilap ilap commented Sep 23, 2024

Description

This pull request implements the tiered fee calculation model introduced in the Cardano Conway era (Chang Hardfork).

Related Issue(s)

Closes #870

Description of Changes

  • Replaced the existing LinearFee model with a TieredFee model that applies progressively higher fees for reference script sizes beyond the initial 25KiB.
  • Renamed minNoScriptFee() to minFee() for better clarity.
  • Added a method to calculate the length of reference scripts, contributing to the overall fee calculation.
  • Introduced the ReferenceScriptSizeLimitExceededException class to handle cases where the reference script size exceeds defined limits.
  • Added tests for the TieredFee class to ensure that it correctly calculates fees for transactions with reference scripts.
  • Added unit tests for length calculations and fee calculations involving reference scripts.

Breaking Changes

  • The renaming of minNoScriptFee() to minFee() may require adjustments in dependent modules that reference this method.
  • The inputs field has been updated from List<TransactionUnspentOutput> to Set<TransactionUnspentOutput>. This may require adjustments in dependent code that expects a List instead of a Set.

Other than that, there are no significant breaking changes.

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

ilap and others added 4 commits September 23, 2024 13:15
…DL alignment and change referenceInputs for reference script fee calculation
…'s classes for reference script fee calculation
…n for reference scripts

  * Replace LinearFee with TieredFee to support tiered fee calculation for transactions that consider reference scripts.
  * Rename the minNoScriptFee() method to minFee() for clarity.
  * Make the script constructors private and add a fromHex(String cborHex) factory constructor for PlutusScripts.
  * Add tests for the length property of scripts, which also validates the fromHex factory constructor.
  * Add tests for fee calculation with reference scripts.
  * Add a ReferenceScriptSizeLimitExceededException class for handling errors when the reference script size limit is exceeded.
@stevenj stevenj changed the title feat(catalyst_cardano_serialization): Implement tiered fee calculation for reference scripts feat(dart/catalyst_cardano_serialization): Implement tiered fee calculation for reference scripts Sep 24, 2024
@dtscalac dtscalac self-requested a review September 24, 2024 06:29
@dtscalac dtscalac added review me PR is ready for review dart Pull requests that update Dart code labels Sep 24, 2024
@dtscalac
Copy link
Contributor

@ilap Thanks for the PR. Make sure to denote in the PR's title that it contains breaking changes. I updated this PR title.

@dtscalac dtscalac changed the title feat(dart/catalyst_cardano_serialization): Implement tiered fee calculation for reference scripts feat(dart/catalyst_cardano_serialization)!: Implement tiered fee calculation for reference scripts Sep 24, 2024
@dtscalac
Copy link
Contributor

@ilap The changes in catalyst_cardano_serialization have caused compile errors in other dependent packages, i.e. catalyst_cardano. These packages provide different functionalities, they build on top of catalyst_cardano_serialization but in short, we use them as a test for backward compatibility when changing the serialization package.

Would it be possible to adjust these compilation issues as well?

…sizeIncrement, maxRefScriptSize) to props for equality
…es caused by catalyst_cardano_serialization changes
Copy link
Contributor

@dtscalac dtscalac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@stevenj Please support on merging the PR, I validated it successfully.

@stevenj stevenj merged commit 28cb3c5 into input-output-hk:main Sep 24, 2024
1 of 2 checks passed
@stevenj
Copy link
Collaborator

stevenj commented Sep 24, 2024

@ilap Thank you again for the great contribution.

@ilap
Copy link
Contributor Author

ilap commented Jan 2, 2025

Hi @stevenj and @dtscalac,

Could you please update the version of the catalyst_cardano_serialization package and any related dependencies, and publish them on pub.dev?

Thanks in advance!

Cheers,
Pal

@dtscalac
Copy link
Contributor

dtscalac commented Jan 2, 2025

Hi @stevenj and @dtscalac,

Could you please update the version of the catalyst_cardano_serialization package and any related dependencies, and publish them on pub.dev?

Thanks in advance!

Cheers, Pal

Hi @ilap, thanks for reaching out. You can find the newly updates packages here: pub.dev

@ilap
Copy link
Contributor Author

ilap commented Jan 6, 2025

Hi @stevenj and @dtscalac,
Could you please update the version of the catalyst_cardano_serialization package and any related dependencies, and publish them on pub.dev?
Thanks in advance!
Cheers, Pal

Hi @ilap, thanks for reaching out. You can find the newly updates packages here: pub.dev

Hi guys,

It seems the frb is not fully configured for the published catalyst_key_derivation package, as I ran into error when I tried to use it in some flutter example.

  Compiling catalyst_key_derivation v0.1.0 (......./catalyst-voices/catalyst_voices/packages/libs/catalyst_key_derivation/rust)
error[E0583]: file not found for module `frb_generated`
  --> src/lib.rs:20:1
   |
20 | mod frb_generated;
   | ^^^^^^^^^^^^^^^^^^
   |
   = help: to create the module `frb_generated`, create file "src/frb_generated.rs" or "src/frb_generated/mod.rs"
   = note: if there is a `mod frb_generated` elsewhere in the crate already, import it with `use crate::...` instead

error[E0432]: unresolved import `crate::frb_generated::FLUTTER_RUST_BRIDGE_HANDLER`
  --> src/api/key_derivation/mod.rs:14:5
   |
14 | use crate::frb_generated::FLUTTER_RUST_BRIDGE_HANDLER;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `FLUTTER_RUST_BRIDGE_HANDLER` in `frb_generated`

Some errors have detailed explanations: E0432, E0583.
For more information about an error, try `rustc --explain E0432`.
error: could not compile `catalyst_key_derivation` (lib) due to 2 previous errors

@dtscalac
Copy link
Contributor

dtscalac commented Jan 7, 2025

@ilap can you share your sample? Was it a web project or android/ios?

@ilap
Copy link
Contributor Author

ilap commented Jan 7, 2025

@ilap can you share your sample? Was it a web project or android/ios?

Hey @dtscalac,

We should continue this discussion in the issue #1471

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart Pull requests that update Dart code review me PR is ready for review
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Feature]: Implement tiered fee calculation for the Conway era
3 participants