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

Evaluator fix #97

Merged
merged 8 commits into from
Oct 17, 2024
Merged

Evaluator fix #97

merged 8 commits into from
Oct 17, 2024

Conversation

lisicky
Copy link
Collaborator

@lisicky lisicky commented Oct 13, 2024

Summary

This pull request introduces several improvements and fixes to the sidan-csl-rs crate, particularly in the areas of transaction script evaluation and network type handling. The changes include passing references instead of values, adding error text for better debugging, fixing network deserialization, and introducing new tests to ensure the reliability of these changes.

Type of Change

Feature Change

  • WASM sidan-csl-rs - Feature update

Maintenance

  • Bug fix (non-breaking change which fixes an issue)
  • Code refactoring (improving code quality without changing its behavior)

Checklist

  • My code is appropriately commented and includes relevant documentation, if necessary
  • I have added tests to cover my changes, if necessary
  • I have updated the documentation, if necessary
  • All new and existing tests pass
  • Both rust and wasm build pass

Additional Information

The main changes in this pull request include:

  1. Modified evaluate_tx_scripts_js function to pass resolved_utxos and additional_txs by reference, improving memory management.
  2. Improved error handling and messages in UTXO and network deserialization.
  3. Implemented TryFrom<String> for Network enum to enhance network type parsing.
  4. Added as_ref_vec method to JsVecString for better reference handling.
  5. Introduced new tests for network type decoding and UTXO/TX evaluation to ensure the reliability of these changes.

@lisicky lisicky changed the title Evaluator Evaluator fix Oct 13, 2024
@lisicky lisicky marked this pull request as ready for review October 17, 2024 08:41
@HinsonSIDAN
Copy link
Member

image

@lisicky any ideas what this linting about? actually probably can ignore for now but want to see if you have thoughts

@lisicky
Copy link
Collaborator Author

lisicky commented Oct 17, 2024

image @lisicky any ideas what this linting about? actually probably can ignore for now but want to see if you have thoughts

@HinsonSIDAN it’s about that the function is not used. We can drop this function, but I would left it for future

@HinsonSIDAN
Copy link
Member

Ok i make it a publicly accessible function for now.

Copy link
Member

@HinsonSIDAN HinsonSIDAN left a comment

Choose a reason for hiding this comment

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

lgtm

@HinsonSIDAN HinsonSIDAN merged commit 12c6f6c into master Oct 17, 2024
3 checks passed
@lisicky lisicky deleted the fuature/evaluator branch October 17, 2024 10:07
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.

2 participants