Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Move process_instruction defs to runtime#12507

Merged
jackcmay merged 4 commits intosolana-labs:masterfrom
jackcmay:move-invoke-context-to-runtime
Sep 29, 2020
Merged

Move process_instruction defs to runtime#12507
jackcmay merged 4 commits intosolana-labs:masterfrom
jackcmay:move-invoke-context-to-runtime

Conversation

@jackcmay
Copy link
Copy Markdown
Contributor

Problem

  • Bunch of definitions that don't need to be in the sdk
  • We are going to need to pipe FeatureSet into InvokeContext but FeatureSet is local to runtime

Summary of Changes

These definitions don't need to be in the SDK and moving them into runtime brings them in with their users and allows us to leave FeatureSet where it is.

Fixes #

Comment thread genesis-programs/src/lib.rs Outdated
@@ -7,10 +7,12 @@ extern crate solana_exchange_program;
#[macro_use]
extern crate solana_vest_program;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you rebase on #12490 (review for that PR would be great too!). genesis-programs/... is no more!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@jackcmay jackcmay force-pushed the move-invoke-context-to-runtime branch from c3e770c to f7d89d1 Compare September 25, 2020 23:49
@jackcmay jackcmay force-pushed the move-invoke-context-to-runtime branch from f7d89d1 to 3fcf642 Compare September 28, 2020 20:35
@jackcmay
Copy link
Copy Markdown
Contributor Author

jackcmay commented Sep 28, 2020

@ryoqun Any ideas on how to fix this CI?

Background, I moved types from the sdk into runtime. The bank depends on Executor which was moved. Rust was happy before because AbiExample was in the same crate as impl AbiExample for Arc<dyn Executor >. How to define an AbiExample for a dynamic trait in this case?

Comment thread runtime/src/process_instruction.rs Outdated
) -> Result<(), InstructionError>;
}
#[cfg(RUSTC_WITH_SPECIALIZATION)]
impl AbiExample for Arc<dyn Executor> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ryoqun Here is the code that breaks:

error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
   --> runtime/src/process_instruction.rs:134:1
    |
134 | impl AbiExample for Arc<dyn Executor> {
    | ^^^^^^^^^^^^^^^^^^^^-----------------
    | |                   |
    | |                   `Arc` is not defined in the current crate
    | impl doesn't use only types from inside the current crate
    |
    = note: define and implement a trait or new type instead

error: aborting due to previous error; 1 warning emitted

@jackcmay
Copy link
Copy Markdown
Contributor Author

@ryoqun Thanks!

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 29, 2020

Codecov Report

Merging #12507 into master will decrease coverage by 0.0%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #12507     +/-   ##
=========================================
- Coverage    82.0%    82.0%   -0.1%     
=========================================
  Files         354      354             
  Lines       82643    82636      -7     
=========================================
- Hits        67788    67780      -8     
- Misses      14855    14856      +1     

@jackcmay jackcmay merged commit 2ff9836 into solana-labs:master Sep 29, 2020
@jackcmay jackcmay added the v1.3 label Sep 29, 2020
mergify Bot pushed a commit that referenced this pull request Sep 29, 2020
mergify Bot added a commit that referenced this pull request Sep 29, 2020
(cherry picked from commit 2ff9836)

Co-authored-by: Jack May <jack@solana.com>
@jackcmay jackcmay deleted the move-invoke-context-to-runtime branch April 7, 2021 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants