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.

Sign up for GitHub

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

Jump to bottom

Add command to calculate plutus script costs from an already constructed transaction #1031

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Add command to calculate plutus script costs from an already constructed transaction #1031

merged 1 commit into from
Feb 18, 2025

Conversation

Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented Jan 24, 2025 •
edited by palas
Loading

Changelog

- description: |
    Add command to calculate plutus script costs from an already constructed transaction.
  type:
  - feature

Context

This is a first step towards addressing this issue: #945.

This PR in cardano-api provides the required functionality: IntersectMBO/cardano-api#735. This PR must wait for the functionality in the cardano-api PR to get in a release, and the source repo stanza must be removed before merging.

How to trust this PR

Ensure the untyped bits are correct: serialisation, documentation, and interface design.

Check in conjunction with these PRs:

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

@Jimbo4350 Jimbo4350 linked an issue Jan 24, 2025 that may be closed by this pull request
[FR] - Command to calculate the correct Plutus Script Costs offline #945
Open
Jimbo4350
Jimbo4350 commented Jan 24, 2025
View reviewed changes
Copy link
Contributor Author

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

If you look closely renderScriptCosts could be refactored to accept Map ScriptWitnessIndex ScriptHash as a parameter instead of [(ScriptWitnessIndex, AnyScriptWitness era)].

@palas palas self-assigned this Jan 24, 2025
@palas palas added the enhancement New feature or request label Jan 24, 2025
@palas palas changed the title Plutus script cost calculation cmd Add command to calculate plutus script costs from an already constructed transaction Jan 24, 2025
@palas palas mentioned this pull request Jan 24, 2025
Merged
3 tasks
@palas palas marked this pull request as ready for review January 24, 2025 16:06
carbolymer
carbolymer reviewed Jan 27, 2025
View reviewed changes
Copy link
Contributor

@carbolymer carbolymer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you add a test case in cardano-testnet for that command?

@palas palas force-pushed the plutus-script-cost-calculation-cmd branch 4 times, most recently from 27052c5 to 4d420e9 Compare January 30, 2025 16:16
@palas palas mentioned this pull request Jan 30, 2025
Open
9 tasks
@palas palas removed a link to an issue Jan 31, 2025
[FR] - Command to calculate the correct Plutus Script Costs offline #945
Open
@palas palas mentioned this pull request Jan 31, 2025
Closed
@palas palas linked an issue Jan 31, 2025 that may be closed by this pull request
Command to calculate the correct Plutus Script Costs online #1045
Closed
@palas palas force-pushed the plutus-script-cost-calculation-cmd branch 2 times, most recently from 874c63c to 0d54cd9 Compare January 31, 2025 17:13
@palas palas force-pushed the plutus-script-cost-calculation-cmd branch 3 times, most recently from 8bd3765 to 3604f0c Compare January 31, 2025 18:16
Jimbo4350
Jimbo4350 commented Feb 4, 2025
View reviewed changes
Copy link
Contributor Author

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to close this PR and re-open a new one so it is rightly counted as yours.

Oh and clean up the git history please!

@palas palas changed the base branch from master to bump-cardano-api-10.9.0.0 February 17, 2025 12:37
@palas palas force-pushed the plutus-script-cost-calculation-cmd branch 3 times, most recently from 4675b4b to 8b79453 Compare February 17, 2025 15:18
@palas palas force-pushed the bump-cardano-api-10.9.0.0 branch from f43b65c to 1c89380 Compare February 17, 2025 15:21
@palas palas force-pushed the plutus-script-cost-calculation-cmd branch from 8b79453 to 5a856d6 Compare February 17, 2025 15:28
Base automatically changed from bump-cardano-api-10.9.0.0 to master February 17, 2025 21:40
@palas palas force-pushed the plutus-script-cost-calculation-cmd branch from 5a856d6 to e69011f Compare February 18, 2025 09:41
@palas palas enabled auto-merge February 18, 2025 09:44
@palas palas added this pull request to the merge queue Feb 18, 2025
Merged via the queue into master with commit 299bcd7 Feb 18, 2025
26 checks passed
@palas palas deleted the plutus-script-cost-calculation-cmd branch February 18, 2025 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carbolymer carbolymer carbolymer approved these changes

@palas palas palas approved these changes

@newhoggy newhoggy Awaiting requested review from newhoggy newhoggy is a code owner

@smelc smelc Awaiting requested review from smelc smelc is a code owner

@kevinhammond kevinhammond Awaiting requested review from kevinhammond kevinhammond is a code owner

@disassembler disassembler Awaiting requested review from disassembler disassembler is a code owner

Assignees

@palas palas

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Command to calculate the correct Plutus Script Costs online
3 participants
@Jimbo4350 @carbolymer @palas