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

BPF to WASM #52

Closed
wants to merge 27 commits into from
Closed

BPF to WASM #52

wants to merge 27 commits into from

Conversation

sviatoslav-alekseev
Copy link
Collaborator

@sviatoslav-alekseev sviatoslav-alekseev commented Mar 17, 2023

Solana has BPF smart contracts allowing write programs in Rust and C. We need to add WASM support for smart contracts. With this change, we could write smart contracts in any language that supports WASM.

What is being done:

  • copied programs/bpf_loader to programs/wasm_loader
  • for every BPFLoader program add an identical WASMLoader
  • add WASM smart contract loader with wasmi library
  • link WASM module to BPF syscalls

@sviatoslav-alekseev sviatoslav-alekseev self-assigned this Mar 17, 2023
lamports_to_sol(fee),
*from_pubkey,
));
// TODO(Dev): this is only for speed up development.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we resolve this TODO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted this part

*account_pubkey,
));
}
// TODO(Dev): this is only for speed up development.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we resolve this TODO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted this part

pre_warp_slot,
todo!(),
))
todo!();
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted this part

@@ -129,4 +129,4 @@ fn bench_serialize_aligned(bencher: &mut Bencher) {
bencher.iter(|| {
let _ = serialize_parameters_aligned(&transaction_context, instruction_context).unwrap();
});
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks odd, should we revert this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted

@@ -258,13 +259,13 @@ fn write_program_data(
invoke_context: &mut InvokeContext,
) -> Result<(), InstructionError> {
let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?;
let instruction_context = transaction_context.get_current_instruction_context().inspect_err(|x| { dbg!(x); })?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question whether we need to add all these inspect_err

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleted all inspect_err

@@ -155,7 +155,12 @@ pub fn register_syscalls(
)?;

// Logging
syscall_registry.register_syscall_by_name(b"sol_log_", SyscallLog::init, SyscallLog::call)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we revert it if it's only about formating?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted

Copy link

@fishseabowl fishseabowl left a comment

Choose a reason for hiding this comment

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

If possible, could you add some description to help others understand this PR? For example, what is the goal of this PR? What are the main data structures and interfaces of this PR? Thanks

@@ -963,7 +981,8 @@ impl<'a> InvokeContext<'a> {
err
});
} else {
return (entry.process_instruction)(first_instruction_account, self);
return (entry.process_instruction)(first_instruction_account, self)
.inspect_err(|x| { dbg!(x, entry); });

Choose a reason for hiding this comment

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

error[E0658]: use of unstable library feature 'result_option_inspect'
--> program-runtime/src/invoke_context.rs:985:26
|
985 | .inspect_err(|x| { dbg!(x, entry); });
| ^^^^^^^^^^^
|
= note: see issue #91345 rust-lang/rust#91345 for more information

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleted all inspect_err

return (entry.process_instruction)(first_instruction_account, self)
.inspect_err(|x| { dbg!(x, &entry, &first_instruction_account); })

Choose a reason for hiding this comment

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

error[E0658]: use of unstable library feature 'result_option_inspect'
--> program-runtime/src/invoke_context.rs:975:26
|
975 | ... .inspect_err(|x| { dbg!(x, &entry, &first_instruction_account); })
| ^^^^^^^^^^^
|
= note: see issue #91345 rust-lang/rust#91345 for more information

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleted all inspect_err

@sviatoslav-alekseev
Copy link
Collaborator Author

If possible, could you add some description to help others understand this PR? For example, what is the goal of this PR? What are the main data structures and interfaces of this PR? Thanks

@fishseabowl Here is a description for this PR:
#52 (comment)

@fishseabowl
Copy link

Hi sviatoslav-alekseev, I got some warnings when I compiled your PR 52, I don't think it's because of this PR, but can you fix it?

warning: the type `[iovec; 64]` does not permit being left uninitialized
  --> streamer/src/recvmmsg.rs:78:52
   |
78 | ...= unsafe { mem::MaybeUninit::uninit().assume_init() };
   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |               |
   |               this code causes undefined behavior when executed
   |               help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
   |
   = note: raw pointers must be initialized
   = note: `#[warn(invalid_value)]` on by default

warning: the type `iovec` does not permit being left uninitialized
   --> streamer/src/sendmmsg.rs:137:34
    |
137 | ...unsafe { std::mem::MaybeUninit::uninit().assume_init() }; size];
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |             |
    |             this code causes undefined behavior when executed
    |             help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
    |
    = note: raw pointers must be initialized
warning: unreachable expression
   --> core/src/cluster_info_vote_listener.rs:618:13
    |
617 |             todo!(),
    |             ------- any code following this expression is unreachable
618 |             &root_bank,
    |             ^^^^^^^^^^ unreachable expression
    |
    = note: `#[warn(unreachable_code)]` on by default

warning: unused variable: `root_bank`
   --> core/src/cluster_info_vote_listener.rs:604:9
    |
604 |         root_bank: &Bank,
    |         ^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_root_bank`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `bank_forks`
   --> core/src/cluster_info_vote_listener.rs:605:9
    |
605 |         bank_forks: Arc<RwLock<BankForks>>,
    |         ^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_bank_forks`

warning: unused variable: `subscriptions`
   --> core/src/cluster_info_vote_listener.rs:606:9
    |
606 |         subscriptions: &RpcSubscriptions,
    |         ^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_subscriptions`

warning: unused variable: `gossip_verified_vote_hash_sender`
   --> core/src/cluster_info_vote_listener.rs:607:9
    |
607 |         gossip_verified_vote_hash_sender: &GossipVerifiedVoteHashSender,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_gossip_verified_vote_hash_sender`

warning: unused variable: `verified_vote_sender`
   --> core/src/cluster_info_vote_listener.rs:608:9
    |
608 |         verified_vote_sender: &VerifiedVoteSender,
    |         ^^^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_verified_vote_sender`

warning: unused variable: `replay_votes_receiver`
   --> core/src/cluster_info_vote_listener.rs:609:9
    |
609 |         replay_votes_receiver: &ReplayVoteReceiver,
    |         ^^^^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_replay_votes_receiver`

warning: unused variable: `bank_notification_sender`
   --> core/src/cluster_info_vote_listener.rs:610:9
    |
610 |         bank_notification_sender: &Option<BankNotificationSender>,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_bank_notification_sender`

warning: unused variable: `cluster_confirmed_slot_sender`
   --> core/src/cluster_info_vote_listener.rs:611:9
    |
611 |         cluster_confirmed_slot_sender: &Option<GossipDuplicateConfirmedS...
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_cluster_confirmed_slot_sender`

warning: unused variable: `total_weight`
   --> core/src/cluster_info_vote_listener.rs:612:9
    |
612 |         total_weight: u64,
    |         ^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_total_weight`

warning: associated function `listen_and_confirm_votes` is never used
   --> core/src/cluster_info_vote_listener.rs:601:8
    |
601 |     fn listen_and_confirm_votes(
    |        ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default
warning: the following packages contain code that will be rejected by a future version of Rust: bindgen v0.59.1, fs_extra v1.2.0, nalgebra v0.27.1, nom v6.1.2
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`

@sviatoslav-alekseev
Copy link
Collaborator Author

Rebased on the master branch. New PR #57

I will try to fix current discussions there.

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.

3 participants