chore: update rust-simplicity dependency#314
Conversation
|
@ivanlele in your opinion are we ready to just release a new version of rust-simplicity with these changes? |
@apoelstra, looks like it, the remaining work for #224 is only on the HL side. I was actually about to ask you what we needed to make release myself |
|
Okay, dope. @ivanlele can you open a PR on rust-simplicity which bumps the version in Cargo.toml and the lockfiles, and adds whatever CHANGELOG.md entries you think are worth mentioning? Then I can merge that, and tag and publish it. |
|
Can we possibly also get BlockstreamResearch/rust-simplicity#362 merged before the tag? |
|
oops yes definitely. let me review it now. |
On it, I'll make it a draft for now, before Mike's PR is reviewed |
stringhandler
force-pushed
the
chore/update-rust-simplicity
branch
from
7d37b65 to
e292d60
Compare
c3f0ce8 general: remove mention of the lsp and vscode (Kyryl R) 9d8c2d3 refactor: remove lsp and vscode (stringhandler) Pull request description: Removes the LSP and VS Code extension from this repo and into https://github.com/BlockstreamResearch/simplicityhl-lsp # Motivation When trying to update `rust-simplicity` in #314 I am unable to get the full project to pass CI. This is because the LSP project references Simplicity via a crates version. As it stands, there are usually multiple PRs to update the LSP after code in the rest of the project is edited. ACKs for top commit: KyrylR: ACK c3f0ce8; successfully ran local tests, docs and review Tree-SHA512: ec7e94d95c0b21683ddeed46a9e678e176490690c96179be76c06e6cc578263ae7cba166700fe432421e8a344eea5a05c2b5b5c6dcfa71a01a7d5223b57fcd99
stringhandler
force-pushed
the
chore/update-rust-simplicity
branch
from
7b44457 to
f028068
Compare
…ew API Switch to master branch of rust-simplicity, which removes the `Jet` type parameter from nodes and renames broken lock distance/ duration jets. Update all call sites to match the new API and deprecate example files that use the renamed jets.
stringhandler
force-pushed
the
chore/update-rust-simplicity
branch
from
f8a1acf to
83f6c50
Compare
|
Finally ready for review |
|
Hah, glad to see the new |
|
Yeah I originally moved these examples to a new deprecated folder, but I decided to do that in a follow up PR instead |
| // The reason we need to advance by a bit is that the AssertL combinator is actually a Case combinator, | ||
| // which takes a bit of input to decide which branch to take. But this bit is "meaningless" and | ||
| // is always 0 because it's an assertion. | ||
| let _ = input_frame.next(); |
There was a problem hiding this comment.
In 83f6c50:
Why remove this from handle_jet here?
There was a problem hiding this comment.
Similar in parse_jet_result below.
There was a problem hiding this comment.
The tests fail with them. I suspect that the extra bit was actually a result of the Frame bug we fixed in rust-simplicity, and that @KyrylR (I suspect he added these) assumed it was the case that the jet had a wrapping case, when it did not.
There was a problem hiding this comment.
Hah, fascinating! I think you are right -- it looks like all this code comes from PRs like #198 with the attitude "let's just patch this til it works for now, and eventually we'll replace this debug-wrapping stuff completely".
| match node.inner() { | ||
| Inner::Jet(jet) => self.handle_jet(node, *jet, &input, &output), | ||
| Inner::Jet(jet) => { | ||
| if let Some(&elements_jet) = jet.as_any().downcast_ref::<Elements>() { |
There was a problem hiding this comment.
In 83f6c50:
I guess we can do this as a followup but I suspect this downcast is unnecessary and we can do everything using the generic jet.
There was a problem hiding this comment.
Ah yeah, let me look at this again.
There was a problem hiding this comment.
handle_jet will be updated later to accept dynamic jets, so this downcast will disappear down the line.
| Inner::Jet(jet) => { | ||
| if let Some(&elements_jet) = jet.as_any().downcast_ref::<Elements>() { | ||
| self.handle_jet(node, elements_jet, &input, &output); | ||
| } |
There was a problem hiding this comment.
In 83f6c50:
While we have the downcast, let's add an else { panic! } here so that we visibly crash rather than silently ignoring the jet, if we are given a non-elements program.
| - The result of [`input_utxos_hash`] (32 bytes)."#, | ||
| // Time locks | ||
| Elements::CheckLockDistance => r#"**Deprecated; do not use.** Assert that the value returned by [`tx_lock_distance`] is greater than or equal to the given value. | ||
| Elements::BrokenDoNotUseCheckLockDistance => r#"**Deprecated; do not use.** Assert that the value returned by [`tx_lock_distance`] is greater than or equal to the given value. |
There was a problem hiding this comment.
nit: generated jet module docs still link to the old timelock jet names after the rename (i.e. to [tx_lock_distance], and to [tx_lock_duration] below)
4 participants
Switch to latest master branch of rust-simplicity, which removes the
Jettype parameter from nodes and renames broken lock distance/ duration jets. Update all call sites to match the new API and deprecate example files that use the renamed jets.