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

Add confirmed transaction lookups and always track roots #3749

Merged
merged 2 commits into from
Apr 12, 2019

Conversation

sagar-solana
Copy link
Contributor

@sagar-solana sagar-solana commented Apr 12, 2019

Problem

Nodes use bleeding edge banks for all RPC APIs. This results in some apis reporting different results on different nodes

Summary of Changes

Ensure the root is always updated (even when no voting keypair is provided) and only provide confirmed blockhashes to rpc clients.
Fixes #3442

@sagar-solana sagar-solana requested a review from mvines April 12, 2019 19:04
@sagar-solana
Copy link
Contributor Author

@mvines, I tried this with 3 nodes running off of my PC and it seems to work. Can you help verify blockstreamer ?

@codecov
Copy link

codecov bot commented Apr 12, 2019

Codecov Report

Merging #3749 into master will increase coverage by <.1%.
The diff coverage is 71.4%.

@@           Coverage Diff            @@
##           master   #3749     +/-   ##
========================================
+ Coverage    79.3%   79.3%   +<.1%     
========================================
  Files         155     155             
  Lines       24946   24951      +5     
========================================
+ Hits        19791   19796      +5     
  Misses       5155    5155

Copy link
Contributor

@mvines mvines left a comment

Choose a reason for hiding this comment

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

lgtm

runtime/src/bank.rs Outdated Show resolved Hide resolved
@@ -867,6 +874,13 @@ impl Bank {
pub fn transaction_count(&self) -> u64 {
self.transaction_count.load(Ordering::Relaxed) as u64
}
pub fn confirmed_transaction_count(&self) -> u64 {
if let Some(bank) = self.parents().last() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this one-liner work instead?

self.parents().last().unwrap_or(self).transaction_count()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly, we've got &self and each parent is &Arc

@sagar-solana sagar-solana marked this pull request as ready for review April 12, 2019 21:40
@sagar-solana sagar-solana added the automerge Merge this Pull Request automatically once CI passes label Apr 12, 2019
@solana-grimes solana-grimes merged commit 1e8f83a into solana-labs:master Apr 12, 2019
@sagar-solana sagar-solana deleted the fix_blockstreamer branch June 14, 2019 04:57
joncinque pushed a commit to joncinque/solana that referenced this pull request Nov 26, 2024
* Add CODEOWNERS for SVM-related directories

* Remove whitespace for sanity check
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getRecentBlockhash JSON RPC API returns different values from different nodes in testnet-beta
3 participants