Skip to content

Allow Plutus cost calculation to take offline data instead of querying the node #1079

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

Merged
merged 4 commits into from
Apr 25, 2025

Conversation

palas
Copy link
Contributor

@palas palas commented Mar 5, 2025

Changelog

- description: |
    Modified Plutus cost calculation command to allow the user to supply offline data instead of querying the node
  type:
  - feature

Context

This PR addresses this issue: #945
This PR depends on the following PR from cardano-api, I would recommend looking at both in conjunction: IntersectMBO/cardano-api#771

How to trust this PR

The test should provide some confidence, there is not a lot of change in the actual implementation, except that the parameters allow to provide the required info directly instead of querying the node.

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

@palas palas self-assigned this Mar 5, 2025
@palas palas linked an issue Mar 5, 2025 that may be closed by this pull request
@palas palas marked this pull request as ready for review March 6, 2025 15:46
@palas palas force-pushed the query-interpreter branch 2 times, most recently from f98a9d8 to 885e91f Compare March 6, 2025 15:54
@palas palas force-pushed the query-interpreter branch from 838a241 to 685d8c8 Compare March 12, 2025 12:10
@palas palas requested a review from Jimbo4350 March 12, 2025 12:12
@palas palas force-pushed the query-interpreter branch 2 times, most recently from 690efe9 to 8a480f8 Compare April 10, 2025 16:03
Copy link
Contributor

@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.

Nice work. After this round of commits and tidying up the commit history this is good to go.

@palas palas requested review from carbolymer and Jimbo4350 and removed request for carbolymer April 14, 2025 18:48
@palas palas force-pushed the query-interpreter branch from 19942ba to ad7b1a8 Compare April 15, 2025 18:57
@palas palas requested a review from newhoggy April 15, 2025 18:57
@palas palas force-pushed the query-interpreter branch from ad7b1a8 to 7078030 Compare April 15, 2025 18:59
Copy link
Contributor

@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.

  • No need for the metavar changes to be in this PR. Please remove them to a separate PR and open an ADR.
  • This needs to be addresses: #1079 (comment)
  • Introduce online vs offline as pointed out in slack

Everything else looks good. Happy to approve after.

@palas palas force-pushed the query-interpreter branch from 7078030 to c93a53a Compare April 21, 2025 12:12
@palas palas requested review from a team as code owners April 21, 2025 12:12
@palas palas force-pushed the query-interpreter branch 2 times, most recently from 25e013e to 299861f Compare April 21, 2025 12:39
@palas palas requested a review from Jimbo4350 April 21, 2025 16:48
@Jimbo4350 Jimbo4350 self-requested a review April 22, 2025 13:59
palas added 4 commits April 25, 2025 18:18
Make `--out-file` optional

Add support for eras before conway
- Split online and offline functionality into subcommands
- Use point free style to improve code layout
- Replace `evalEitherM` and `readJsonFile` with `readJsonFileOk`
- Remove parametrisation on era

Co-authored-by: Mateusz Galazyn
<[email protected]>
@palas palas force-pushed the query-interpreter branch from 5e415e4 to 6e22c35 Compare April 25, 2025 16:19
@palas palas dismissed carbolymer’s stale review April 25, 2025 16:20

Already addressed issues and agreed the PR was ready

@palas palas enabled auto-merge April 25, 2025 16:30
@palas palas added this pull request to the merge queue Apr 25, 2025
Merged via the queue into master with commit 8b8d15a Apr 25, 2025
25 checks passed
@palas palas deleted the query-interpreter branch April 25, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] - Command to calculate the correct Plutus Script Costs offline
5 participants