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

Filter out stake and vote accounts with incorrect owners#14062

Merged
CriesofCarrots merged 5 commits intosolana-labs:masterfrom
CriesofCarrots:filter-accounts-by-owner
Dec 11, 2020
Merged

Filter out stake and vote accounts with incorrect owners#14062
CriesofCarrots merged 5 commits intosolana-labs:masterfrom
CriesofCarrots:filter-accounts-by-owner

Conversation

@CriesofCarrots
Copy link
Copy Markdown
Contributor

Problem

Follow up to #13615
While the stake program now checks whether a vote account is correctly owned on Delegation, the runtime method to collect stake and vote accounts doesn't verify whether this is still the case.

Summary of Changes

Filter out invalid stake and vote accounts

Comment thread runtime/src/bank.rs
Comment thread runtime/src/bank.rs
Comment thread runtime/src/bank.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 11, 2020

Codecov Report

Merging #14062 (b336e84) into master (f02d4cd) will increase coverage by 0.0%.
The diff coverage is 94.0%.

@@           Coverage Diff            @@
##           master   #14062    +/-   ##
========================================
  Coverage    82.1%    82.1%            
========================================
  Files         381      381            
  Lines       94076    94211   +135     
========================================
+ Hits        77291    77407   +116     
- Misses      16785    16804    +19     

Comment thread runtime/src/bank.rs
@ryoqun
Copy link
Copy Markdown
Contributor

ryoqun commented Dec 11, 2020

@CriesofCarrots btw, did you tested this manually?:

Well, I have a secret sauce of easily creating fake account via the wracked solana deploy command:

$ ./target/release/solana --url http://localhost:8899 account -o vote.data <foo bar pubkey json>
$ hexedit
$ ./target/release/solana.bad-deploy deploy --url http://localhost:8899/ vote.data <foo bar pubkey json>
diff --git a/cli/src/cli.rs b/cli/src/cli.rs
index aa7b69fb7c..e1ee8aa66b 100644
--- a/cli/src/cli.rs
+++ b/cli/src/cli.rs
@@ -1227,8 +1227,8 @@ fn do_process_deploy(
         CliError::DynamicProgramError(format!("Unable to read program file: {}", err))
     })?;
 
-    Executable::<BPFError>::from_elf(&program_data, Some(|x| bpf_verifier::check(x, true)))
-        .map_err(|err| CliError::DynamicProgramError(format!("ELF error: {}", err)))?;
+    //Executable::<BPFError>::from_elf(&program_data, Some(|x| bpf_verifier::check(x, true)))
+    //    .map_err(|err| CliError::DynamicProgramError(format!("ELF error: {}", err)))?;
 
     let loader_id = if use_deprecated_loader {
         bpf_loader_deprecated::id()

this is not strict requirement, though. :)

ryoqun
ryoqun previously approved these changes Dec 11, 2020
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun left a comment

Choose a reason for hiding this comment

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

lgtm after metrics addition and small test tweaks!

Really thanks for taking this over. :)

@ryoqun
Copy link
Copy Markdown
Contributor

ryoqun commented Dec 11, 2020

Well, I have a secret sauce...

@CriesofCarrots And this for faster inflation:

SOLANA_RUN_SH_CLUSTER_TYPE=mainnet-beta/development SOLANA_RUN_SH_GENESIS_ARGS="--slots-per-epoch 128 --vote-commission-percentage=55" ./run.sh

@mergify mergify Bot dismissed ryoqun’s stale review December 11, 2020 07:42

Pull request has been modified.

@CriesofCarrots
Copy link
Copy Markdown
Contributor Author

@ryoqun thanks for the special sauce! It took a few more steps, but I was able to test this manually. Merging!

@CriesofCarrots CriesofCarrots merged commit d6eff3d into solana-labs:master Dec 11, 2020
mergify Bot pushed a commit that referenced this pull request Dec 11, 2020
* Add failing test

* Check stake/vote accounts for validity

* Feature gate change

* Add datapoint

* Add test realism

(cherry picked from commit d6eff3d)
CriesofCarrots added a commit that referenced this pull request Dec 12, 2020
* Add failing test

* Check stake/vote accounts for validity

* Feature gate change

* Add datapoint

* Add test realism

(cherry picked from commit d6eff3d)
mergify Bot added a commit that referenced this pull request Dec 12, 2020
…4080)

* Add failing test

* Check stake/vote accounts for validity

* Feature gate change

* Add datapoint

* Add test realism

(cherry picked from commit d6eff3d)

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
@CriesofCarrots CriesofCarrots deleted the filter-accounts-by-owner branch December 16, 2020 20:42
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.

2 participants