PR 2 of 3: Add bindings in simplicity-sys crate. #49

sanket1729 · 2022-12-14T09:01:00Z

Based on #48

sanket1729 · 2022-12-15T11:51:21Z

Rebased on top of merged #48 . Add a commit that fixes the filenames

uncomputable · 2022-12-15T12:53:04Z

Not sure if something went wrong during rebasing, but the first commits 81dfee2 ... 9f55381 have the same hashes as existing commits on master.

sanket1729 · 2022-12-15T13:05:24Z

@uncomputable, those were merged as a part of the first PR. This PR now only consists of three commits.

EDIT: Sorry, you were right. I had to rebase on master, not on top of first PR because of the merge commit.

simplicity-sys/depend/wrapper.c

simplicity-sys/src/c_jets/c_env.rs

simplicity-sys/src/c_jets/frame_ffi.rs

simplicity-sys/src/c_jets/c_frame.rs

simplicity-sys/depend/env.c

sanket1729 · 2022-12-16T09:04:29Z

Resolved all comments.

Force pushed with

Using assertions to check the alignment is working as intended.
Marking new_read and new_write as unsafe
Using a pointer to usize instead of u8. C code has a pointer to UWORD, but that should map to usize in on all 32/64 bit machines. This is also checked in tests and will be checked before running any jet

sanket1729 · 2022-12-16T09:17:52Z

Also pushed #50 that shows how this code is used. Refer to the file here: https://github.com/ElementsProject/rust-simplicity/blob/8eb9d1af989b709bd173a70fb92d5e547a89d5f4/src/jet/mod.rs#L132-L171

apoelstra · 2022-12-17T16:01:14Z

This looks great now!

apoelstra

ACK 987ade1

bbbb7ca Clippy warnings (sanket1729) 0a853b0 Use FFI jets instead of regular jets (sanket1729) b4a3064 Remove depcrecated jets (sanket1729) f5c3070 rename to_type() -> to_variable_type (sanket1729) a7e2cc8 Prepare for adding jets: Comment policy compiler (sanket1729) Pull request description: Based on #48 and #49 ACKs for top commit: apoelstra: ACK bbbb7ca Tree-SHA512: 56f867d6e31bdb9abfdf43b171887a3a3dca3b8141d43fc807d328a041ec2a4d2351b8af918b205cac1366dee7815239a2a8f722b89c393f0f693b83af1b29ec

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.

405487e ffi: move Raw* structs out of FFI crate (Andrew Poelstra) c42a375 ffi: don't use empty enums for opaque types (Andrew Poelstra) 2d8c56e ffi: remove c_set_rawElementsTapEnv (Andrew Poelstra) 4bc03bf ffi: remove c_set_rawElementsTransaction (Andrew Poelstra) b0e8484 ffi: remove c_set_rawBuffer method (Andrew Poelstra) 5dab248 ffi: delete the c_set_rawElementsInput method (Andrew Poelstra) 32c66e0 ffi: store pegin genesis hash in RawInputData (Andrew Poelstra) 224645a ffi: delete the c_set_rawElementsOutput method (Andrew Poelstra) 818b380 ffi: replace some "pointer to assumed length" types with optional references (Andrew Poelstra) e23a1b3 ffi: don't flatten CRawInput structure (Andrew Poelstra) 66a468c simplicity-sys: delete dead links to C code (Andrew Poelstra) a925cd9 simplicity-sys: move all the elements FFI stuff into its own module (Andrew Poelstra) 78568e6 simplicity_sys: rename c_env.rs to c_env/mod.rs (Andrew Poelstra) Pull request description: Currently our transaction environment construction is quite complicated, and (as a consequence) unsound in a couple of places. This was implemented in PRs #49 through #52 in the early days of this crate with very little review, using a strategy copied from Haskell but not really appropriate for Rust. In particular: * We do not need custom marshalling functions in Rust because we can directly define `repr(C)` structs * ...in doing so, we may use Rust references in place of raw pointers, as long as the C code upholds the Rust aliasing rules (which it does, at least for these read-only POD types); in fact we may use optional references in place of nullable pointers * ...and we can use arrays and pointers to arrays in place of pointers to primitive types which are supposed to point to a specific number of elements * ...but we must make sure that any data we produce a pointer or reference to outlives the pointer/reference! There was at least one instance where we were creating a temporary variable and extracting a pointer from it, which would then outlive the object it pointed into. (This occurred for the annex, though in dead code, and for the pegin genesis hash, in live code that has never been used.) As an aside, we used empty enums in a couple places for opaque types. We should not do this, according to the nomicon. ACKs for top commit: canndrew: ACK 405487e Tree-SHA512: 76e08be787fdfa99ccbe3778cf5553c671fa8b556f071658e9dd1c58c0d0dff8aa1a132b77f3e12eea018dbdb47906e0c0e4d3135c53948a2472d4d16c927237

This was referenced Dec 14, 2022

PR 3 of 3: Use bindings created for jet execution. #50

Merged

Jets FFI #47

Closed

sanket1729 force-pushed the add_bindings branch from 36f4fce to 4c38dce Compare December 15, 2022 11:50

Rename filenames

df61fe0

sanket1729 force-pushed the add_bindings branch from 4c38dce to 59c2942 Compare December 15, 2022 13:05