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

request response for get transactions & sealed header from p2p network #919

Merged
merged 43 commits into from
Jan 24, 2023

Conversation

leviathanbeak
Copy link
Contributor

@leviathanbeak leviathanbeak commented Jan 19, 2023

closes #878 #877

The work has been done on 2 (now merged) branches, by @freesig and myself.

@freesig freesig changed the base branch from master to leviathanbeak/heartbeat_protocol January 20, 2023 04:30
Base automatically changed from leviathanbeak/heartbeat_protocol to master January 23, 2023 07:47
@leviathanbeak leviathanbeak changed the title request response for get transactions from p2p network request response for get transactions & sealed header from p2p network Jan 23, 2023
@leviathanbeak leviathanbeak marked this pull request as ready for review January 23, 2023 17:52
@leviathanbeak leviathanbeak requested a review from a team January 23, 2023 17:53
@leviathanbeak leviathanbeak linked an issue Jan 23, 2023 that may be closed by this pull request
@leviathanbeak
Copy link
Contributor Author

Note: the PR is not that big, it contains small refactors and better naming apart from the needed changes

@ControlCplusControlV ControlCplusControlV requested a review from a team January 23, 2023 18:29
leviathanbeak and others added 4 commits January 23, 2023 20:31
…:FuelLabs/fuel-core into leviathanbeak/get_transactions_from_peer
Co-authored-by: ControlCplusControlV <44706811+ControlCplusControlV@users.noreply.github.com>
@freesig freesig enabled auto-merge (squash) January 23, 2023 20:11
@@ -1275,7 +1287,7 @@ mod tests {
tokio::select! {
message_sent = rx_test_end.recv() => {
// we received a signal to end the test
assert_eq!(message_sent, Some(true), "Received wrong block height!");
assert_eq!(message_sent, Some(true), "Receuved incorrect or missing missing messsage");
Copy link
Contributor

@ControlCplusControlV ControlCplusControlV Jan 23, 2023

Choose a reason for hiding this comment

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

[nit]

Suggested change
assert_eq!(message_sent, Some(true), "Receuved incorrect or missing missing messsage");
assert_eq!(message_sent, Some(true), "Received incorrect or missing missing messsage");

Assertion seems to be failing here, although it make sense with p2p being disabled that p2p failed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is something wrong with Request::Transactions test others are passing, if no one solves it by tomorrow I will try it.

It seems the request doesn't even go through, the node gets disconnected every time I re-ran it with Transactions, other requests pass which is (slightly) weird

@ControlCplusControlV ControlCplusControlV requested a review from a team January 23, 2023 21:41
@freesig freesig merged commit 84c2ebf into master Jan 24, 2023
@freesig freesig deleted the leviathanbeak/get_transactions_from_peer branch January 24, 2023 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[P2P] GetBlockTransactions [P2P] GetSealedBlockHeader
5 participants