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

Make extrinsics #[transactional] #576

Merged
merged 8 commits into from
May 14, 2022

Conversation

maltekliemann
Copy link
Member

@maltekliemann maltekliemann commented Apr 20, 2022

We make all extrinsics #[transactional] unless they are not already explicitly marked as non-transactional. We now use MARK(non-transactional): description... as format for marking extrinsics which use verify-first-write-second.

Fixes zeitgeistpm/runtime-audit-1#17, fixes zeitgeistpm/runtime-audit-1#18, fixes zeitgeistpm/runtime-audit-1#24 and fixes zeitgeistpm/runtime-audit-2#7.

@maltekliemann maltekliemann added the s:in-progress The pull requests is currently being worked on label Apr 20, 2022
@maltekliemann maltekliemann added this to the v0.3.2 milestone Apr 20, 2022
@maltekliemann
Copy link
Member Author

maltekliemann commented Apr 20, 2022

Fixes zeitgeistpm/runtime-audit-1#17, zeitgeistpm/runtime-audit-1#18, zeitgeistpm/runtime-audit-1#24 and fixes zeitgeistpm/runtime-audit-2#7.

@maltekliemann
Copy link
Member Author

I get the following error:

warning: Error finalizing incremental compilation session directory `/Users/malte/zeitgeist-fork-2/target/debug/incremental/zrml_prediction_markets-3bhssma6hsrml/s-g91jr0e40v-1hsiw8r-working`: No such file or directory (os error 2)

error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: broken MIR in DefId(0:1226 ~ zrml_prediction_markets[da45]::pallet::{impl#0}::create_scalar_market::{closure#0}::{closure#0}) (((*(_1.4: &[u8; 50])).0: [u8; 50])): can't project out of PlaceTy { ty: [u8; 50], variant_index: None }
   --> zrml/prediction-markets/src/lib.rs:591:37
    |
591 |             let MultiHash::Sha3_384(multihash) = metadata;
    |                                     ^^^^^^^^^
    |
    = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:860:31

error: internal compiler error: TyKind::Error constructed but no error reported
  |
  = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:795:20

error: internal compiler error: TyKind::Error constructed but no error reported
  |
  = note: delayed at /rustc/52ca603da73ae9eaddf96f77953b33ad8c47cc8e/compiler/rustc_middle/src/ty/relate.rs:414:59

error: internal compiler error: broken MIR in DefId(0:1217 ~ zrml_prediction_markets[da45]::pallet::{impl#0}::create_scalar_market) ((_4.0: [u8; 50])): can't project out of PlaceTy { ty: zeitgeist_primitives::types::MultiHash, variant_index: None }
   --> zrml/prediction-markets/src/lib.rs:570:9
    |
570 |         #[transactional]
    |         ^^^^^^^^^^^^^^^^
    |
    = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:860:31
    = note: this error: internal compiler error originates in the attribute macro `transactional` (in Nightly builds, run with -Z macro-backtrace for more info)

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1383:13
stack backtrace:
   0: std::panicking::begin_panic::<rustc_errors::ExplicitBug>
   1: std::panic::panic_any::<rustc_errors::ExplicitBug>
   2: <rustc_errors::HandlerInner as core::ops::drop::Drop>::drop
   3: core::ptr::drop_in_place::<rustc_session::parse::ParseSess>
   4: <alloc::rc::Rc<rustc_session::session::Session> as core::ops::drop::Drop>::drop
   5: core::ptr::drop_in_place::<rustc_interface::interface::Compiler>
   6: rustc_span::with_source_map::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_interface::interface::create_compiler_and_run<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#1}>
   7: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.62.0-nightly (52ca603da 2022-04-12) running on x86_64-apple-darwin

note: compiler flags: --crate-type lib -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 -C incremental

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
warning: `zrml-prediction-markets` (lib) generated 2 warnings
error: could not compile `zrml-prediction-markets`; 2 warnings emitted

@maltekliemann
Copy link
Member Author

maltekliemann commented Apr 21, 2022

I'm putting this up for review. If the review goes through, I'll add an issue for the removal of the workaround. I'm also working on a minimization of the problem so we can submit a bug report for the ICE.

I'm going through with the workaround. The issue is #608.

@maltekliemann maltekliemann added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Apr 21, 2022
@maltekliemann
Copy link
Member Author

A minimization of the compiler bug is reported in rust-lang/rust#96299.

Copy link
Member

@sea212 sea212 left a comment

Choose a reason for hiding this comment

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

LGTM

zrml/liquidity-mining/src/lib.rs Outdated Show resolved Hide resolved
zrml/swaps/src/lib.rs Show resolved Hide resolved
@sea212 sea212 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels May 14, 2022
Co-authored-by: Harald Heckmann <mail@haraldheckmann.de>
@maltekliemann maltekliemann merged commit 0518c7e into zeitgeistpm:main May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:accepted This pull request is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants