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

Use last_valid_block_height in services and client apps #19163

Merged
merged 6 commits into from
Aug 11, 2021

Conversation

CriesofCarrots
Copy link
Contributor

Problem

last_valid_slot is deprecated, and all clusters now support block-height RPCs.

Summary of Changes

Update SendTransactionService, solana program, and solana-tokens CLI to use last_valid_block_height

@CriesofCarrots
Copy link
Contributor Author

@mvines , can you please look at the SendTransactionService changes? I don't think there should be any breakage for anyone, but would appreciate the sanity check.
Also, should I include BanksClient/BanksServer? Those changes will be more breaking (updating get_fees_with_commitment_and_context to return a block height, instead of a slot). But maybe nbd, if those crates are only being used for program tests.

jackcmay
jackcmay previously approved these changes Aug 10, 2021
@mvines
Copy link
Member

mvines commented Aug 10, 2021

@mvines , can you please look at the SendTransactionService changes? I don't think there should be any breakage for anyone, but would appreciate the sanity check.

lgtm

Also, should I include BanksClient/BanksServer? Those changes will be more breaking (updating get_fees_with_commitment_and_context to return a block height, instead of a slot). But maybe nbd, if those crates are only being used for program tests.

yeah that's probably the right thing to do even though it's a little breaky. It'll only affect program tests so shouldn't be too painful 🤞🏼

@mergify mergify bot dismissed jackcmay’s stale review August 11, 2021 04:23

Pull request has been modified.

@codecov
Copy link

codecov bot commented Aug 11, 2021

Codecov Report

Merging #19163 (de0d4b8) into master (fd93754) will increase coverage by 0.0%.
The diff coverage is 89.5%.

@@           Coverage Diff            @@
##           master   #19163    +/-   ##
========================================
  Coverage    82.9%    82.9%            
========================================
  Files         450      453     +3     
  Lines      128336   129053   +717     
========================================
+ Hits       106411   107102   +691     
- Misses      21925    21951    +26     

@CriesofCarrots CriesofCarrots merged commit 5970083 into solana-labs:master Aug 11, 2021
mergify bot pushed a commit that referenced this pull request Aug 11, 2021
* Add deprecated tag to Bank::get_blockhash_last_valid_slot

* Update SendTransactionService to use last_valid_block_height

* Update solana-tokens to use last_valid_block_height

* Remove dangling file

* Update solana program to use last_valid_block_height

* Update Banks crates to use last_valid_block_height

(cherry picked from commit 5970083)

# Conflicts:
#	cli/src/program.rs
mergify bot added a commit that referenced this pull request Aug 11, 2021
) (#19171)

* Use last_valid_block_height in services and client apps (#19163)

* Add deprecated tag to Bank::get_blockhash_last_valid_slot

* Update SendTransactionService to use last_valid_block_height

* Update solana-tokens to use last_valid_block_height

* Remove dangling file

* Update solana program to use last_valid_block_height

* Update Banks crates to use last_valid_block_height

(cherry picked from commit 5970083)

# Conflicts:
#	cli/src/program.rs

* Fix conflict

Co-authored-by: Tyera Eulberg <[email protected]>
Co-authored-by: Tyera Eulberg <[email protected]>
@CriesofCarrots CriesofCarrots deleted the use-lvbh branch August 12, 2021 17:41
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
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