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

Add new builtin hashing function ripemd_160 #6147

Closed
wants to merge 2 commits into from
Closed

Add new builtin hashing function ripemd_160 #6147

wants to merge 2 commits into from

Conversation

paluh
Copy link
Contributor

@paluh paluh commented May 29, 2024

Kenneth created a comprehensive ticket for that addition: #6155

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Changelog fragments have been written (if appropriate)
    • Relevant tickets are mentioned in commit messages
    • Formatting, PNG optimization, etc. are updated
  • PR
    • (For external contributions) Corresponding issue exists and is linked in the description
    • Targeting master unless this is a cherry-pick backport
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@paluh
Copy link
Contributor Author

paluh commented May 29, 2024

This is preliminary work which exposes RIPEMD-160 hashing primitive as a new bulitin.

  • I closely followed changes which were made during introduction of keccak_256 and blake2b_224 functions.
  • The current version of the PR depends on a branch of cardano-base which exposes this primitive hence the changes in cabal.project
  • This draft contains an artificial costing model for the new builtin - I copied keccak_256 related values to be able to test the function.

@paluh paluh changed the title Add new builtin hashing function ripemd-160 Add new builtin hashing function ripemd_160 May 29, 2024
@@ -0,0 +1 @@
(program 1.0.0 (con bool True))
Copy link
Contributor

Choose a reason for hiding this comment

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

JFYI: I advocate for not using golden file machinery for programs that are expected to evaluate to a simple value like boolean True and instead verify expected result directly in the test code.

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'm happy to refactor this out if that is the way to go. Could you please point me to a place where I should put such a test case?

Copy link
Contributor

Choose a reason for hiding this comment

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

JFYI: I advocate for not using golden file machinery for programs that are expected to evaluate to a simple value like boolean True and instead verify expected result directly in the test code.

I don't think that necessarily applies here. This is in the conformance tests and by design all of those are golden tests whose input and output are textual UPLC so that they can be run by other evaluators, in particular by the Agda evaluator in plutus-metatheory.

@paluh paluh closed this Jun 27, 2024
@paluh
Copy link
Contributor Author

paluh commented Jun 27, 2024

I'm closing this in favor of: #6252 because of administrative issues (repo permissions).

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.

3 participants