Skip to content

[remote-executor] Isolate wormhole-solana crate #843

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
May 26, 2023
Merged

[remote-executor] Isolate wormhole-solana crate #843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fa8b418
Point to another wormhole sdk
guibescos May 24, 2023
94c447a
Works
guibescos May 25, 2023
9c25f6e
Reset this dir
guibescos May 25, 2023
bcf423e
Refresh cargo lock
guibescos May 25, 2023
fa58fb5
Bump solana
guibescos May 25, 2023
60d18fc
Pin git packages
guibescos May 25, 2023
01152dd
Allow large errors
guibescos May 25, 2023
4402790
Update to the right wormhole-solana
guibescos May 25, 2023
6eb0b4d
Fix cli
guibescos May 25, 2023
08beb57
Add cargo lock
guibescos May 25, 2023
839e1dd
Remove print statement
guibescos May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/remote-executor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
override: true
- name: Install Solana
run: |
sh -c "$(curl -sSfL https://release.solana.com/stable/install)"
sh -c "$(curl -sSfL https://release.solana.com/v1.14.18/install)"
Copy link
Contributor Author

@guibescos guibescos May 25, 2023

Choose a reason for hiding this comment

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

I had to bump this

echo "/home/runner/.local/share/solana/install/active_release/bin" >> $GITHUB_PATH
- name: Run executor tests
run: cargo test-bpf --manifest-path ./governance/remote_executor/Cargo.toml
94 changes: 80 additions & 14 deletions governance/remote_executor/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions governance/remote_executor/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ members = [

[profile.release]
overflow-checks = true

[patch.crates-io]
serde_wormhole = { git = "https://github.com/wormhole-foundation/wormhole", tag = "v2.17.1" }
Copy link
Contributor Author

@guibescos guibescos May 25, 2023

Choose a reason for hiding this comment

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

This patch.crate-io wormhole thing is getting out of control, wormhole needs to fix

Copy link
Contributor

@Reisen Reisen May 26, 2023

Choose a reason for hiding this comment

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

Agreed, reasonable fix for now though. Let's ask for publishing.

5 changes: 3 additions & 2 deletions governance/remote_executor/cli/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ anchor-client = "0.25.0"
shellexpand = "2.1.2"
anyhow = "1.0.65"
base64 = "0.13.0"
wormhole-solana = { git = "https://github.com/guibescos/wormhole", branch = "reisen/sdk-solana"}
wormhole-core = { git = "https://github.com/guibescos/wormhole", branch = "reisen/sdk-solana"}
wormhole-solana = { git = "https://github.com/guibescos/wormhole-solana", rev="f14b3b54c1e37e1aaf8c2ac2a5e236832ffdb3c2"}
wormhole-sdk = { git = "https://github.com/wormhole-foundation/wormhole", tag = "v2.17.1" }
serde_wormhole = { git = "https://github.com/wormhole-foundation/wormhole", tag = "v2.17.1"}
hex = "0.4.3"
33 changes: 21 additions & 12 deletions governance/remote_executor/cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
#![deny(warnings)]

use {
serde_wormhole::RawMessage,
wormhole_sdk::vaa::{
Body,
Header,
Vaa,
},
};
pub mod cli;
use {
anchor_client::{
Expand Down Expand Up @@ -47,7 +56,6 @@ use {
transaction::Transaction,
},
std::str::FromStr,
wormhole::VAA,
wormhole_solana::{
instructions::{
post_message,
Expand Down Expand Up @@ -86,9 +94,10 @@ fn main() -> Result<()> {

let signature_set_keypair = Keypair::new();

let vaa = VAA::from_bytes(vaa_bytes.clone())?;
let vaa: Vaa<&RawMessage> = serde_wormhole::from_slice(&vaa_bytes)?;
let (header, body): (Header, Body<&RawMessage>) = vaa.into();
Copy link
Contributor

@Reisen Reisen May 26, 2023

Choose a reason for hiding this comment

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

Nice nice nice.


let posted_vaa_key = PostedVAA::key(&wormhole, vaa.digest().unwrap().hash);
let posted_vaa_key = PostedVAA::key(&wormhole, body.digest().unwrap().hash);

// First verify VAA
let verify_txs = verify_signatures_txs(
Expand All @@ -106,15 +115,15 @@ fn main() -> Result<()> {

// Post VAA
let post_vaa_data = PostVAAData {
version: vaa.version,
guardian_set_index: vaa.guardian_set_index,
timestamp: vaa.timestamp,
nonce: vaa.nonce,
emitter_chain: vaa.emitter_chain.into(),
emitter_address: vaa.emitter_address,
sequence: vaa.sequence,
consistency_level: vaa.consistency_level,
payload: vaa.payload,
version: header.version,
guardian_set_index: header.guardian_set_index,
timestamp: body.timestamp,
nonce: body.nonce,
emitter_chain: body.emitter_chain.into(),
emitter_address: body.emitter_address.0,
sequence: body.sequence,
consistency_level: body.consistency_level,
payload: body.payload.to_vec(),
};

process_transaction(
Expand Down
6 changes: 4 additions & 2 deletions governance/remote_executor/programs/remote-executor/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ pythtest = []

[dependencies]
anchor-lang = {version = "0.25.0", features = ["init-if-needed"]}
wormhole-solana = { git = "https://github.com/guibescos/wormhole", branch = "reisen/sdk-solana"}
wormhole-core = { git = "https://github.com/guibescos/wormhole", branch = "reisen/sdk-solana"}
wormhole-solana = { git = "https://github.com/guibescos/wormhole-solana", rev="f14b3b54c1e37e1aaf8c2ac2a5e236832ffdb3c2"}
Copy link
Contributor

@Reisen Reisen May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We should really get away from this, maybe we can chat about what needs to be upstreamed. We're running into this in pythnet too. I believe this was based on my original PR that had plans to revive, we should push for it if possible.

Copy link
Contributor Author

@guibescos guibescos May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is a temporary home.
Two options from here :

  • Its own repo (maybe switch from my personal github to wormhole or pyth org)
  • Make it part of sdk/rust/ in the wormhole monorepo

wormhole-sdk = { git = "https://github.com/wormhole-foundation/wormhole", tag = "v2.17.1" }
serde_wormhole = { git = "https://github.com/wormhole-foundation/wormhole", tag = "v2.17.1"}
boolinator = "2.4.0"

[dev-dependencies]

solana-program-test = "=1.10.31"
tokio = "1.14.1"
solana-sdk = "=1.10.31"
Expand Down
3 changes: 2 additions & 1 deletion governance/remote_executor/programs/remote-executor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![deny(warnings)]
#![allow(clippy::result_unit_err)]
#![allow(clippy::result_large_err)]

use {
anchor_lang::{
Expand All @@ -12,7 +13,7 @@ use {
claim_record::ClaimRecord,
posted_vaa::AnchorVaa,
},
wormhole::Chain::{
wormhole_sdk::Chain::{
self,
Solana,
},
Expand Down
2 changes: 1 addition & 1 deletion governance/remote_executor/programs/remote-executor/src/state/governance_payload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
mem::size_of,
ops::Deref,
},
wormhole::Chain,
wormhole_sdk::Chain,
};

pub const MAGIC_NUMBER: u32 = 0x4d475450; // Reverse order of the solidity contract because borsh uses little endian numbers (the solidity contract uses 0x5054474d)
Expand Down
17 changes: 11 additions & 6 deletions governance/remote_executor/programs/remote-executor/src/tests/executor_simulator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use {
ToAccountMetas,
},
solana_program_test::{
find_file,
read_file,
BanksClient,
BanksClientError,
Expand All @@ -54,8 +53,11 @@ use {
TransactionError,
},
},
std::collections::HashMap,
wormhole::Chain,
std::{
collections::HashMap,
path::Path,
},
wormhole_sdk::Chain,
wormhole_solana::VAA,
};

Expand All @@ -81,9 +83,12 @@ pub enum VaaAttack {
impl ExecutorBench {
/// Deploys the executor program as upgradable
pub fn new() -> ExecutorBench {
let mut bpf_data = read_file(find_file("remote_executor.so").unwrap_or_else(|| {
panic!("Unable to locate {}", "remote_executor.so");
}));
let mut bpf_data = read_file(
std::env::current_dir()
.unwrap()
.join(Path::new("../../target/deploy/remote_executor.so")),
);
Copy link
Contributor Author

@guibescos guibescos May 25, 2023

Choose a reason for hiding this comment

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

Newer versions of solana can't find the binary with the previous logic

Copy link
Contributor

@Reisen Reisen May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can you explain why? Inherently suspicious of needing relative paths.

Copy link
Contributor Author

@guibescos guibescos May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In previous versions (1.13.x) when you ran cargo test-bpf, it would set an environment variable BPF_OUT_DIR and that's how the code would find the binary.
New versions (1.14.x) don't seems to set it, so find_file fails.

Maybe it has to do with them trying to migrate to test-sbf and deprecating test-bpf.

Copy link
Contributor

@Reisen Reisen May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I recall the BPF_OUT_DIR issues as well. Can you mention what might be the reason it's not doing it before we merge this? Don't worry if it takes too much time you can merge over this comment, it would just be nice to know what changed here for other solana related work.



let mut program_test = ProgramTest::default();
let program_key = crate::id();
Expand Down
2 changes: 2 additions & 0 deletions governance/remote_executor/rust-toolchain.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[toolchain]
channel = "1.66.1"
Copy link
Contributor Author

@guibescos guibescos May 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Drive-by pin toolchain

Loading