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

Downgrade Rust to 1.67.0 #856

Closed
wants to merge 2 commits into from
Closed

Downgrade Rust to 1.67.0 #856

wants to merge 2 commits into from

Conversation

doubledup
Copy link
Contributor

Opened as a draft until it's successfully built locally.

This avoids an issue where the sign-ext WASM feature is enabled in Rust >= 1.70.0.

cargo-contract issue
rust issue

This avoids an issue where the sign-ext feature is enabled in Rust >=
1.70.0.

[cargo-contract issue](use-ink/cargo-contract#1139)
[rust issue](rust-lang/rust#109807)
@doubledup doubledup self-assigned this Jun 12, 2023
Copy link
Contributor

@claravanstaden claravanstaden left a comment

Choose a reason for hiding this comment

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

🥳 Thanks @doubledup!

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Patch coverage has no change and project coverage change: +1.50 🎉

Comparison is base (cd9e049) 73.10% compared to head (b483682) 74.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #856      +/-   ##
==========================================
+ Coverage   73.10%   74.61%   +1.50%     
==========================================
  Files          38       28      -10     
  Lines        1517     1292     -225     
  Branches       52        0      -52     
==========================================
- Hits         1109      964     -145     
+ Misses        388      328      -60     
+ Partials       20        0      -20     

see 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ascjones
Copy link

You guys are using ink pallet-contracts? 👀

@doubledup
Copy link
Contributor Author

1.69.0 gives the same opcode error. Trying 1.67.0 nightly...

@vgeddes
Copy link
Collaborator

vgeddes commented Jun 12, 2023

You guys are using ink pallet-contracts? 👀

Hi @ascjones, nope we aren't using ink contracts. We were just referencing other projects which were also having issues with bleeding edge rust.

@vgeddes
Copy link
Collaborator

vgeddes commented Jun 12, 2023

@yrong is this downgrade also fine for you? It seems all the eth light client checks above have passed.

@yrong
Copy link
Contributor

yrong commented Jun 12, 2023

is this downgrade also fine for you? It seems all the eth light client checks above have passed.

I can not reproduce the issue though. But since we've box wrapped PreparedPublicKey the previous issue paritytech/substrate#14075 does not block me anymore. So I'm fine if the downgrade can solve problem here.

@doubledup doubledup changed the title Downgrade Rust to 1.69.0 Downgrade Rust to 1.67.0 Jun 13, 2023
@vgeddes
Copy link
Collaborator

vgeddes commented Jun 14, 2023

As said on Slack, I want us to try 1.70 stable and see if that works for us.

@yrong
Copy link
Contributor

yrong commented Jun 14, 2023

I want us to try 1.70 stable and see if that works for us.

I did test with 1.70 stable and just work without problem. But as use-ink/cargo-contract#1139 suggested 1.69 maybe safer to start with.

@doubledup
Copy link
Contributor Author

Closing this in favour of #855

@doubledup doubledup closed this Jun 14, 2023
@doubledup doubledup deleted the david/avoid-sign-ext branch June 14, 2023 08:37
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.

5 participants