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

Fix panic while decoding large Miniscripts from Script #712

Merged

Conversation

sanket1729
Copy link
Member

@sanket1729 sanket1729 commented Jul 21, 2024

Use private module to guard all constructors.

@apoelstra
Copy link
Member

The new lockfiles don't work with MSRV.

@apoelstra
Copy link
Member

ACK 54c7c2b other than the lockfile commit.

apoelstra added a commit that referenced this pull request Jul 23, 2024
32bd66c Release 10.2 (Sanket Kanjalkar)
708b89c Fix panic while decoding large Miniscripts from Script (Sanket Kanjalkar)

Pull request description:

ACKs for top commit:
  apoelstra:
    ACK 32bd66c

Tree-SHA512: 946ea92a680cec1a7ccfc78138a4dbefed7421aac013f68bee3730882741eb1319a8ea3d351fe663d8881e84f0108460a65476e4371d65359e8a968ffe3b806b
apoelstra added a commit that referenced this pull request Jul 23, 2024
fb6e8e9 Release 9.2 (Sanket Kanjalkar)
a290a61 Fix panic while decoding large Miniscripts from Script (Sanket Kanjalkar)

Pull request description:

ACKs for top commit:
  apoelstra:
    utACK fb6e8e9; did test basic compilation on 1.41.1

Tree-SHA512: f384cefe15a2bca9de79447d038dba1f178e2e680931abe195f672160c3233088277fda287ce509181b47b0633daddb320e784e4d774309f1332e8cf434a343b
@sanket1729 sanket1729 force-pushed the sanketk/24-07/script-parse branch 2 times, most recently from 45bcd1a to 74e81ef Compare July 29, 2024 19:19
@sanket1729
Copy link
Member Author

@apoelstra this is ready for review.

@apoelstra
Copy link
Member

The new lockfile updates cc to 1.1.6 which still does not work with MSRV.

Ok if I force-push to this PR to try to fix the lockfiles?

@sanket1729
Copy link
Member Author

@apoelstra, please do. I was assuming that CI is happy means we are good

@sanket1729
Copy link
Member Author

@apoelstra I just tested it again with 1.56.1 and it works.

image

@apoelstra
Copy link
Member

Re "CI happy means we're good", see #719 -- we are actually testing only with Cargo-minimal.lock, not Cargo-recent.lock (which is broken).

Furthermore, this lockfile commit makes several unnecessary changes and because it's in a separate commit from the one that changes Cargo.toml, also means that I cannot locally test every commit of this PR.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

Successfully ran local tests on 192c499.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 192c499 successfully ran local tests

@apoelstra apoelstra merged commit 8f54b5e into rust-bitcoin:master Aug 6, 2024
13 checks passed
@apoelstra
Copy link
Member

Tagged and published.

@apoelstra apoelstra mentioned this pull request Aug 7, 2024
apoelstra added a commit that referenced this pull request Aug 7, 2024
c95b411 CI: Run MSRV job with both lock files (Tobin C. Harding)
d8bfe10 Bump MSRV to Rust 1.63.0 (Tobin C. Harding)
f62bdf3 fuzz: Upgrade hongfuzz patch version (Tobin C. Harding)

Pull request description:

  As we have done in `rust-bitcoin` bump the MSVR to Rust 1.63.0

  Also, run the MSRV CI job with both lockfiles (which are currently different).

ACKs for top commit:
  apoelstra:
    ACK c95b411 successfully ran local tests; glad I got #712 in first so I could do the release!

Tree-SHA512: b3ae9583f10ac2d01353736fe68a37bc0fff6774e4ea935b4fbf16bdb3cb033b4c281b8cb45d9e6ead343d2b7033d042190369cbe21b953ac392d19ff564cf1a
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