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

Add replacements for Pubkey::new_rand()/Hash::new_rand()#12987

Merged
mvines merged 5 commits into
solana-labs:masterfrom
mvines:rnd
Oct 22, 2020
Merged

Add replacements for Pubkey::new_rand()/Hash::new_rand()#12987
mvines merged 5 commits into
solana-labs:masterfrom
mvines:rnd

Conversation

@mvines
Copy link
Copy Markdown
Contributor

@mvines mvines commented Oct 19, 2020

Pubkey::new_rand() and Hash::new_rand() are problematic. They are unavailable when compiling solana-sdk with the "program" feature, but will come back when the "everything" (#12985) feature.

Due to how Cargo feature addition works, a program developer could inadvertently add a dependency on Pubkey::new_rand() due to the "everything" feature getting activated in a [dev-dependency] when building native only to later have their build fail when targetting BPF.

When Pubkey and Hash move into a new solana-program-sdk create (that solana-sdk depends on), Pubkey::new_rand() and Hash::new_rand() cannot come along. This will make it impossible to support the new_rand() functions where they currently live.

The migration plan:

  1. Land this PR in master, v1.4 and v1.3
  2. In v1.4, as Pubkey and Hash move into a new solana-program-sdk supply a stub for Pubkey::new_rand() when !BPF for now. Hash::new_rand() is believed to only be in use internally so remove it entirely in v1.4
  3. v1.5/v1.6 timeframe, remove Pubkey::new_rand()

@mvines mvines added the noCI Suppress CI on this Pull Request label Oct 19, 2020
@mvines mvines force-pushed the rnd branch 3 times, most recently from 5a46a4f to 0dff75c Compare October 19, 2020 19:50
@mvines mvines added CI Pull Request is ready to enter CI and removed noCI Suppress CI on this Pull Request labels Oct 20, 2020
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Oct 20, 2020
@mvines mvines force-pushed the rnd branch 6 times, most recently from ce48d71 to 50462d4 Compare October 20, 2020 02:48
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 20, 2020

Codecov Report

Merging #12987 into master will increase coverage by 0.0%.
The diff coverage is 99.1%.

@@           Coverage Diff           @@
##           master   #12987   +/-   ##
=======================================
  Coverage    82.1%    82.1%           
=======================================
  Files         366      366           
  Lines       86237    86295   +58     
=======================================
+ Hits        70878    70932   +54     
- Misses      15359    15363    +4     

@mvines mvines force-pushed the rnd branch 3 times, most recently from 1548d7f to eae525d Compare October 21, 2020 05:23
fn test_parse_account_data() {
let account_pubkey = Pubkey::new_rand();
let other_program = Pubkey::new_rand();
let account_pubkey = solana_sdk::pubkey::new_rand();
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.

Stating solana_sdk everytime is pretty verbose, we would probably want:

use solana_sdk::pubkey;

...

let account_pubkey = pubkey::new_rand();

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.

that was just an artifact of mass search/replace

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.

I'm going to leave this as is, it can be cleaned up later. It'll be a painful manual edit to over a hundred files

@jackcmay
Copy link
Copy Markdown
Contributor

Looks like except one nit

@mvines mvines merged commit 959880d into solana-labs:master Oct 22, 2020
@mvines mvines deleted the rnd branch October 22, 2020 02:08
@mvines mvines added the v1.4 label Oct 22, 2020
mergify Bot added a commit that referenced this pull request Oct 22, 2020
…13076)

* Add pubkey_new_rand(), mark Pubkey::new_rand() deprecated

(cherry picked from commit 0e68ed6)

* Add hash_new_rand(), mark Hash::new_rand() as deprecated

(cherry picked from commit 76f11c7)

* Run `codemod --extensions rs Pubkey::new_rand solana_sdk::pubkey::new_rand`

(cherry picked from commit 7bc073d)

# Conflicts:
#	programs/bpf/benches/bpf_loader.rs
#	runtime/benches/accounts.rs
#	runtime/src/accounts.rs

* Run `codemod --extensions rs Hash::new_rand solana_sdk::hash::new_rand`

(cherry picked from commit 17c3911)

* Remove unused pubkey::Pubkey imports

(cherry picked from commit 959880d)

# Conflicts:
#	runtime/src/accounts_index.rs

* Resolve conflicts

Co-authored-by: Michael Vines <mvines@gmail.com>
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