-
Notifications
You must be signed in to change notification settings - Fork 19
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
Conversation
f98a9d8
to
885e91f
Compare
838a241
to
685d8c8
Compare
690efe9
to
8a480f8
Compare
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.
Nice work. After this round of commits and tidying up the commit history this is good to go.
19942ba
to
ad7b1a8
Compare
ad7b1a8
to
7078030
Compare
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.
- 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.
cardano-cli/test/cardano-cli-golden/files/golden/help/latest_genesis_create-staked.cli
Outdated
Show resolved
Hide resolved
7078030
to
c93a53a
Compare
25e013e
to
299861f
Compare
cardano-cli/test/cardano-cli-test/Test/Cli/Transaction/Build.hs
Outdated
Show resolved
Hide resolved
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]>
5e415e4
to
6e22c35
Compare
Already addressed issues and agreed the PR was ready
Changelog
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#771How 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