PR 3 of 3: Use bindings created for jet execution. #50
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sanket1729
force-pushed
the
use_bindings
branch
from
fae12ef to
e4a37b5
Compare
Closed
sanket1729
force-pushed
the
use_bindings
branch
2 times, most recently
from
911b607 to
f984241
Compare
This module is assuming bitcoin jets that are not implemented/unspecified. Comment this out, this will be renabled later in a separate PR after we have all the jets
Add a new function to_type that outputs Type
These are no longer used in C and probably incorrect. We should remove this code entirely after we use FFI jets. This just removes unneeded fields in the ElementsEnv struct in the next commit
This commit also does a couple other things. 1) Makes modification to bitmachine that allows to skip assertions in test cfg. Useful for creating states of bitmachine wihtout creating a program. 2) Removes all existing jets that were implemented in rust. This is really not a good idea because the definitions of certain jets are hard to emulate and have special corner/wierd cases. We are best to directly use the FFI bindings. 3) Adds test for checking FFI with/without env
Also add clippy.toml to preserve MSRV warnings
sanket1729
force-pushed
the
use_bindings
branch
from
f984241 to
bbbb7ca
Compare
Member
Author
|
Rebased after #49 |
Collaborator
|
We never actually re-enable the |
Collaborator
|
Ah, yes, I see, there's a TODO for doing Bitcoin jets that still needs to be addressed before we can re-enable the module. |
Collaborator
|
Overall this looks great. There are a bunch of At some point we can use mutagen to assess our test coverage. |
apoelstra
approved these changes
Jan 1, 2023
Member
Author
|
@apoelstra, yep. The policy compiler code is really old and based on bitcoin jets that I assumed would be created in 2020. Time to revisit this in 2023. Similar to how we exported the jets, we should also export the test cases from |
apoelstra
added a commit
to apoelstra/rust-simplicity
that referenced
this pull request
Sep 5, 2025
I can't find any citation that this is safe to do. We don't actually use this structure in our Rust code; we populate it by calling a c_set function which is defined on the C side, and then pass it to C functions. So no code needs to be changed. However, I want to remove the c_set_* functions because they hide life information (and in fact, are unsound, as we currently use them) and serve to obfuscate code. They seem to have come from BlockstreamResearch#48 BlockstreamResearch#49 BlockstreamResearch#50 BlockstreamResearch#51 in which Sanket seems to have copied the corresponding Haskell code into Rust, where it is unnecessary and dangerous.
apoelstra
added a commit
to apoelstra/rust-simplicity
that referenced
this pull request
Sep 12, 2025
I can't find any citation that this is safe to do. We don't actually use this structure in our Rust code; we populate it by calling a c_set function which is defined on the C side, and then pass it to C functions. So no code needs to be changed. However, I want to remove the c_set_* functions because they hide life information (and in fact, are unsound, as we currently use them) and serve to obfuscate code. They seem to have come from BlockstreamResearch#48 BlockstreamResearch#49 BlockstreamResearch#50 BlockstreamResearch#51 in which Sanket seems to have copied the corresponding Haskell code into Rust, where it is unnecessary and dangerous.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #48 and #49