Skip to content

wire: Some small hardening and bugfixes on connection logic #454

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Apr 30, 2025
Merged

wire: Some small hardening and bugfixes on connection logic #454

merged 9 commits into from
Apr 30, 2025

Conversation

@Davidson-Souza
Copy link
Member

@Davidson-Souza Davidson-Souza commented Apr 21, 2025 •
edited
Loading

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • New feature
  • Test
  • Other:

Which crates are being modified?

  • floresta-chain
  • floresta-cli
  • floresta-common
  • floresta-compact-filters
  • floresta-electrum
  • floresta-watch-only
  • floresta-wire
  • floresta
  • florestad
  • Other: .

Description

This PR bundles some small fixes on how we select and keep our peers, specially on sync-node. It solves some situations I've found when running floresta for days or weeks non-stop.

Checklist

  • I've signed all my commits
  • I ran just lint
  • I ran cargo test
  • I've checked the integration tests
  • I've followed the contribution guidelines
  • I'm linking the issue being fixed by this PR (if any)

@Davidson-Souza Davidson-Souza added bug Something isn't working code quality Generally improves code readability and maintainability labels Apr 21, 2025
@Davidson-Souza Davidson-Souza added this to the v0.8.0 milestone Apr 21, 2025
jaoleal
jaoleal approved these changes Apr 23, 2025
View reviewed changes
Copy link
Collaborator

@jaoleal jaoleal left a comment

Choose a reason for hiding this comment

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

CACK fa3a0b2

everything running fine locally

JoseSK999
JoseSK999 reviewed Apr 24, 2025
View reviewed changes
@Davidson-Souza
Copy link
Member Author

Updated with @JoseSK999 comments. I've also fixed a few more things I've found running this code. It seems to work well now

@Davidson-Souza Davidson-Souza mentioned this pull request Apr 28, 2025
Merged
21 tasks
@Davidson-Souza
node_context: add new CONNECTION_TIMEOUT
a409e04 
We should timeout connection request faster, because they can clog up
our inflight requests and get us stuck.

I'm also improving readability by adding empty lines between the consts
inside the trait
@Davidson-Souza
address_man: fix a bug with get_peer_to_connect
296063e
@Davidson-Souza
wire: move all timeout logic to node.rs
947ec25
@Davidson-Souza
wire: fix a bug with anchors saving code
580ee74 
The function that saves the anchors was called after all peers were
disconnected. This could cause the function to never see utreexo peers
to save
@Davidson-Souza
wire: add empty lines to help readability
710971d
@Davidson-Souza
addr_man: Don't return connected peers
e66e41f
@Davidson-Souza
Copy link
Member Author

Rebased with master to make sure linting will pass

jaoleal
jaoleal approved these changes Apr 30, 2025
View reviewed changes
@Davidson-Souza Davidson-Souza requested a review from jaoleal April 30, 2025 14:21
JoseSK999
JoseSK999 suggested changes Apr 30, 2025
View reviewed changes
JoseSK999
JoseSK999 reviewed Apr 30, 2025
View reviewed changes
Copy link
Contributor

@JoseSK999 JoseSK999 left a comment

Choose a reason for hiding this comment

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

Some questions

JoseSK999
JoseSK999 approved these changes Apr 30, 2025
View reviewed changes
Copy link
Contributor

@JoseSK999 JoseSK999 left a comment

Choose a reason for hiding this comment

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

ACK 03defdb

JoseSK999
JoseSK999 approved these changes Apr 30, 2025
View reviewed changes
Copy link
Contributor

@JoseSK999 JoseSK999 left a comment

Choose a reason for hiding this comment

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

re-ACK 25300bb

@Davidson-Souza Davidson-Souza merged commit 7345cba into vinteumorg:master Apr 30, 2025
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaoleal jaoleal Awaiting requested review from jaoleal

1 more reviewer

@JoseSK999 JoseSK999 JoseSK999 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working code quality Generally improves code readability and maintainability

Projects

None yet

Milestone

v0.8.0

Development

Successfully merging this pull request may close these issues.

3 participants

@Davidson-Souza @jaoleal @JoseSK999