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

Latest protocol parameters from node 8.1.2 #1122

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

matiwinnetou
Copy link
Contributor

The demo's project protocol parameters is old and incompatible with Hydra 0.13, this a new one which has been tested to work correctly.

@abailly-iohk
Copy link
Contributor

@matiwinnetou It's hard to track the evolution of this file and ensures it stays in sync with the latest node parameters. I think we should rather modify the demo scripts to extract the file from the cardano-node, modify it on the fly with the needed changes to have 0 fees, and use that instead of committing it.

Let's merge this change as is and then plan to update the scripts.

@abailly-iohk
Copy link
Contributor

@matiwinnetou Tests are failing, seems like your changes to pparams had unforeseen side effects.

Copy link
Contributor

@abailly-iohk abailly-iohk left a comment

Choose a reason for hiding this comment

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

Some values are wrong, I think that's what prevent the benchmarks and ETE tests from running

"txFeePerByte": 0,
"utxoCostPerWord": 34488,
"utxoCostPerByte": 4310
"txFeeFixed": 155381,
Copy link
Contributor

Choose a reason for hiding this comment

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

This value must be 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting, I was thinking the default should be main-net costs.

"utxoCostPerWord": 34488,
"utxoCostPerByte": 4310
"txFeeFixed": 155381,
"txFeePerByte": 44,
Copy link
Contributor

Choose a reason for hiding this comment

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

This one too

"utxoCostPerByte": 4310
"txFeeFixed": 155381,
"txFeePerByte": 44,
"utxoCostPerByte": 4310,
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

"txFeeFixed": 155381,
"txFeePerByte": 44,
"utxoCostPerByte": 4310,
"utxoCostPerWord": null
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this null and not 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change this to 0 but node actually returns null. 8.1.2

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔

@abailly-iohk
Copy link
Contributor

I can confirm changing the values of the protocol-parameters.json to 0 fixes the benchmarks.

Copy link
Contributor

@abailly-iohk abailly-iohk left a comment

Choose a reason for hiding this comment

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

@matiwinnetou There's still a parameter that is null instead of 0

"txFeeFixed": 155381,
"txFeePerByte": 44,
"utxoCostPerByte": 4310,
"utxoCostPerWord": null
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this null and not 0?

@abailly-iohk abailly-iohk merged commit 9e3361e into input-output-hk:master Oct 20, 2023
17 checks passed
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.

None yet

2 participants