-
Notifications
You must be signed in to change notification settings - Fork 729
Add last_sortition_ch to RPCTenure #6818
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
base: develop
Are you sure you want to change the base?
Changes from 1 commit
ef021d7
c37416b
4c669a5
5370994
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |
| // | ||
| // You should have received a copy of the GNU General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| use std::net::{IpAddr, Ipv4Addr, SocketAddr}; | ||
|
|
||
| use stacks_common::types::chainstate::ConsensusHash; | ||
|
|
@@ -25,6 +24,47 @@ use crate::net::httpcore::{StacksHttp, StacksHttpRequest}; | |
| use crate::net::test::TestEventObserver; | ||
| use crate::net::ProtocolFamily; | ||
|
|
||
| // A helper function to find two tenures with empty sortitions in between | ||
| pub fn find_sortitions_with_empty_sortitions_between( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we guaranteed to always have this case in the test setup? |
||
| rpc_test: &mut TestRPC, | ||
| ) -> (ConsensusHash, ConsensusHash) { | ||
| // Find two tenures with empty sortitions in bewteen | ||
| let snapshots = rpc_test.peer_1.sortdb().get_all_snapshots().unwrap(); | ||
|
|
||
| let mut first_sortition: Option<&_> = None; | ||
| let mut saw_non_sortition_between = false; | ||
|
|
||
| let mut result: Option<(&_, &_)> = None; | ||
|
|
||
| for s in snapshots.iter() { | ||
| if s.sortition { | ||
| match first_sortition { | ||
| None => { | ||
| first_sortition = Some(s); | ||
| saw_non_sortition_between = false; | ||
| } | ||
| Some(prev) => { | ||
| if saw_non_sortition_between { | ||
| // Found: sortition -> non-sortition(s) -> sortition | ||
| result = Some((prev, s)); | ||
| break; | ||
| } else { | ||
| // restart window | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you also need to clear |
||
| first_sortition = Some(s); | ||
| saw_non_sortition_between = false; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is redundant, we already know this is false here. |
||
| } | ||
| } | ||
| } | ||
| } else if first_sortition.is_some() { | ||
| saw_non_sortition_between = true; | ||
| } | ||
| } | ||
|
|
||
| let (first, second) = result | ||
| .expect("Did not find sortition, non-sortition(s), sortition pattern required for test"); | ||
| (first.consensus_hash.clone(), second.consensus_hash.clone()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_try_parse_request() { | ||
| let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); | ||
|
|
@@ -60,7 +100,7 @@ fn test_try_make_response() { | |
| let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); | ||
|
|
||
| let test_observer = TestEventObserver::new(); | ||
| let rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); | ||
| let mut rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); | ||
|
|
||
| let nakamoto_consensus_hash = rpc_test.consensus_hash.clone(); | ||
|
|
||
|
|
@@ -89,6 +129,11 @@ fn test_try_make_response() { | |
| StacksHttpRequest::new_get_tenure_blocks(addr.clone().into(), &ConsensusHash([0x01; 20])); | ||
| requests.push(request); | ||
|
|
||
| // query tenure with empty sortitions in between | ||
| let (first, second) = find_sortitions_with_empty_sortitions_between(&mut rpc_test); | ||
| let request = StacksHttpRequest::new_get_tenure_blocks(addr.clone().into(), &second); | ||
| requests.push(request); | ||
|
|
||
| let mut responses = rpc_test.run(requests); | ||
|
|
||
| // got the Nakamoto tip | ||
|
|
@@ -97,9 +142,13 @@ fn test_try_make_response() { | |
| "Response:\n{}\n", | ||
| std::str::from_utf8(&response.try_serialize().unwrap()).unwrap() | ||
| ); | ||
|
|
||
| let resp = response.decode_tenure_blocks().unwrap(); | ||
| assert_eq!(resp.consensus_hash, nakamoto_consensus_hash); | ||
| assert_ne!( | ||
| resp.last_sortition_ch, genesis_consensus_hash, | ||
| "Nakamoto tenure's last_sortition_ch should point to the previous winning sortition" | ||
| ); | ||
|
|
||
| let mut blocks_index = 0; | ||
| for block in test_observer.get_blocks().iter().rev() { | ||
| if block.metadata.consensus_hash != nakamoto_consensus_hash { | ||
|
|
@@ -133,6 +182,12 @@ fn test_try_make_response() { | |
| let resp = response.decode_tenure_blocks().unwrap(); | ||
| assert_eq!(resp.consensus_hash, genesis_consensus_hash); | ||
|
|
||
| // genesis/Epoch2 tenure has no parent tenure. Should return an empty consensus hash. | ||
| assert_eq!( | ||
| resp.last_sortition_ch, | ||
| ConsensusHash::from_bytes(&[0u8; 20]).unwrap(), | ||
| ); | ||
|
|
||
| let blocks = test_observer.get_blocks(); | ||
|
|
||
| let block = blocks.first().unwrap(); | ||
|
|
@@ -153,6 +208,17 @@ fn test_try_make_response() { | |
| std::str::from_utf8(&response.try_serialize().unwrap()).unwrap() | ||
| ); | ||
|
|
||
| let (preamble, body) = response.destruct(); | ||
| let (preamble, _body) = response.destruct(); | ||
| assert_eq!(preamble.status_code, 404); | ||
|
|
||
| // got tenure with empty sortitions in between | ||
| let response = responses.remove(0); | ||
| debug!( | ||
| "Response:\n{}\n", | ||
| std::str::from_utf8(&response.try_serialize().unwrap()).unwrap() | ||
| ); | ||
|
|
||
| let resp = response.decode_tenure_blocks().unwrap(); | ||
| assert_eq!(resp.consensus_hash, second); | ||
| assert_eq!(resp.last_sortition_ch, first); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of loading from the sortition DB's highest known canonical tip, can you instead query relative to the sortition identified by the request's consensus hash? That way, it's unambiguous as to what parent consensus hash is being queried (for example, although unlikely, it is possible that the requested sortition and its parent are both non-canonical, and this change will ensure that the correct parent consensus hash is loaded).
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in c37416b. And then redone in 5370994