Skip to content

Conversation

@moisesPompilio
Copy link
Collaborator

Description and Notes

A new integration test has been added to validate the correctness of information exchanged during P2P communication between Floresta and other nodes. The test verifies both the data Floresta sends to peers and the data it receives, ensuring compliance with expected P2P protocol behavior. Specifically, the test checks:

  • Version string accuracy.
  • Advertised services and connection type.
  • Transport protocol and user agent.

This test helps ensure that Floresta correctly identifies itself and interprets peer information during P2P handshakes and ongoing communication.

Additionally, the addnode-v1 and addnode-v2 tests have been refactored to remove unnecessary sleep calls and eliminate redundant code, improving readability and reducing test execution time without affecting correctness.

How to verify the changes you have done?

  • Run the new P2P node information exchange test and confirm that it validates version, services, connection type, transport protocol, and user agent for both outgoing and incoming P2P messages.
  • Review the refactored addnode-v1 and addnode-v2 tests to ensure unnecessary sleeps have been removed and duplicated code has been consolidated.
  • Execute all related integration tests and confirm they pass without issues and run more efficiently than before.

@moisesPompilio moisesPompilio added chore Cleaning, refactoring, reducing complexity code quality Generally improves code readability and maintainability functional tests labels Jan 8, 2026
@moisesPompilio moisesPompilio self-assigned this Jan 8, 2026
@Davidson-Souza
Copy link
Member

tests/florestad/node-info.py looks a lot like tests/floresta-cli/getpeerinfo.py what's the difference?

@moisesPompilio
Copy link
Collaborator Author

tests/florestad/node-info.py looks a lot like tests/floresta-cli/getpeerinfo.py what's the difference?

This came up as a reference from this comment here #733 (comment). In this case I wanted to make a test to see if the handshake information when doing p2p communication between nodes is storing information correctly. So this node-info test is to get only this information and check if it's correct. In the case of the floresta-cli test we have a lot of comparison between bitcoin core and floresta RPC, but this type of information we can't compare since they would be different. So my idea was to put a test in florestad that gets the information that floresta passes to another node and checks that the information floresta receives from the other node is correct. The getpeerinfo test would only check if the information that floresta displays is correct, like field names and information for example. The node-info only wants to know about the information itself.

@Davidson-Souza
Copy link
Member

Could you update after #777? There was a bug in the user agent that should be fixed now

@moisesPompilio moisesPompilio force-pushed the add-test-check-propeties-external-node branch from fdbe901 to a3ebc99 Compare January 12, 2026 22:46
@moisesPompilio
Copy link
Collaborator Author

Could you update after #777? There was a bug in the user agent that should be fixed now

Ok, I adjusted it like #777 did.

@moisesPompilio moisesPompilio force-pushed the add-test-check-propeties-external-node branch 2 times, most recently from 315a1f1 to 1e0f45e Compare January 14, 2026 14:30
@moisesPompilio
Copy link
Collaborator Author

315a1f1 Fixed flaky test failures caused by nodes failing to start on the first attempt. Implemented retry logic (up to 3 attempts) when starting nodes, similar to what the corepc library does. Testing shows no more errors.

Also added utility functions to check node connectivity that work across all variants, centralizing this logic so individual tests no longer need to implement it themselves.

Implement a retry mechanism (max 3 attempts) for node startup. During
integration tests, the first startup attempt often fails, but retrying
typically resolves the issue.
Add a test to verify both the information Floresta sends to peers and the
information it receives during P2P connections. The test checks:

- Version string accuracy.
- Advertised services and connection type.
- Transport protocol and user agent.
Introduce universal helper functions for checking and waiting for peer connections.
These functions work with all node variants (florestad, utreexod, and bitcoind).

Node class additions:
- `get_connection_info()`: Retrieves connection information for a node,
including user_agent and p2p address, used to verify peer connectivity
- `is_peer_connected()`: Checks whether a specific peer appears in the
node's peer list

FlorestaTestFramework class additions:
- `check_connection()`: Verifies that two peers are connected or disconnected
to each other (i.e., each appears in the other's peer list)
- `wait_for_peers_connections()`: Waits up to 15 seconds for two peers to
establish or drop a connection

UtreexodRpc class addition:
- `ping()`: I added the function so that all variants have this functionality.
Refactor the addnode-v1 and addnode-v2 tests to remove unnecessary sleep
calls, improve readability, and eliminate redundant code.
@moisesPompilio moisesPompilio force-pushed the add-test-check-propeties-external-node branch from 1e0f45e to 06047cf Compare January 19, 2026 17:19
Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

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

ACK 06047cf

@jaoleal
Copy link
Collaborator

jaoleal commented Jan 21, 2026

ACK 06047cf

@Davidson-Souza Davidson-Souza merged commit 2b9c45f into getfloresta:master Jan 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Cleaning, refactoring, reducing complexity code quality Generally improves code readability and maintainability functional tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants