feat/ibc_connection_module_queries #322
Conversation
…e. Included example scripts and unit tests
WalkthroughThe recent update enhances the IBC Connection module with support for various queries using asynchronous gRPC calls. It also improves cookie management logic and updates import paths in the Changes
Recent Review DetailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Out of diff range and nitpick comments (2)
tests/core/ibc/connection/grpc/test_ibc_connection_grpc_api.py (1)
21-23: Consider using a more descriptive name for the fixtureibc_connection_servicerto reflect its purpose more clearly.tests/core/ibc/client/grpc/test_ibc_client_grpc_api.py (1)
8-8: Consider adding a comment above the import statement to explain why the import path was changed, especially if it was due to architectural changes or improvements.
| class TestIBCConnectionGrpcApi: | ||
| @pytest.mark.asyncio | ||
| async def test_fetch_connection( | ||
| self, | ||
| ibc_connection_servicer, | ||
| ): | ||
| version = ibc_connection.Version( | ||
| identifier="connection-0", | ||
| features=["ORDER_ORDERED", "ORDER_UNORDERED"], | ||
| ) | ||
| prefix = ibc_commitment.MerklePrefix(key_prefix=b"prefix") | ||
| counterparty = ibc_connection.Counterparty( | ||
| client_id="07-tendermint-0", | ||
| connection_id="connection-0", | ||
| prefix=prefix, | ||
| ) | ||
| connection = ibc_connection.ConnectionEnd( | ||
| client_id="07-tendermint-0", | ||
| versions=[version], | ||
| state=3, | ||
| counterparty=counterparty, | ||
| delay_period=5, | ||
| ) | ||
| proof = b"proof" | ||
| proof_height = ibc_client.Height( | ||
| revision_number=1, | ||
| revision_height=2, | ||
| ) | ||
|
|
||
| ibc_connection_servicer.connection_responses.append( | ||
| ibc_connection_query.QueryConnectionResponse( | ||
| connection=connection, | ||
| proof=proof, | ||
| proof_height=proof_height, | ||
| ) | ||
| ) | ||
|
|
||
| api = self._api_instance(servicer=ibc_connection_servicer) | ||
|
|
||
| result_connection = await api.fetch_connection(connection_id=counterparty.connection_id) | ||
| expected_connection = { | ||
| "connection": { | ||
| "clientId": connection.client_id, | ||
| "versions": [{"identifier": version.identifier, "features": version.features}], | ||
| "state": ibc_connection.State.Name(connection.state), | ||
| "counterparty": { | ||
| "clientId": counterparty.client_id, | ||
| "connectionId": counterparty.connection_id, | ||
| "prefix": {"keyPrefix": base64.b64encode(prefix.key_prefix).decode()}, | ||
| }, | ||
| "delayPeriod": str(connection.delay_period), | ||
| }, | ||
| "proof": base64.b64encode(proof).decode(), | ||
| "proofHeight": { | ||
| "revisionNumber": str(proof_height.revision_number), | ||
| "revisionHeight": str(proof_height.revision_height), | ||
| }, | ||
| } | ||
|
|
||
| assert result_connection == expected_connection |
There was a problem hiding this comment.
The test case test_fetch_connection is well-structured and covers the basic functionality of fetching a connection. However, consider adding negative test cases to handle scenarios where the connection does not exist or the server returns an error.
Would you like me to help by adding some negative test cases?
| import asyncio | ||
|
|
||
| from google.protobuf import symbol_database | ||
|
|
||
| from pyinjective.async_client import AsyncClient | ||
| from pyinjective.core.network import Network | ||
|
|
||
|
|
||
| async def main() -> None: | ||
| network = Network.testnet() | ||
| client = AsyncClient(network) | ||
|
|
||
| connection_id = "connection-0" | ||
| revision_number = 0 | ||
| revision_height = 7379538 | ||
|
|
||
| state = await client.fetch_ibc_connection_consensus_state( | ||
| connection_id=connection_id, revision_number=revision_number, revision_height=revision_height | ||
| ) | ||
| print(state) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| symbol_db = symbol_database.Default() | ||
| asyncio.get_event_loop().run_until_complete(main()) |
There was a problem hiding this comment.
Check for potential file duplication or incorrect content.
This script appears identical to 5_ConnectionConsensusState.py. Please verify if this is intentional or if there has been a mistake in file naming or content duplication.
| from collections import deque | ||
|
|
||
| from pyinjective.proto.ibc.core.connection.v1 import ( | ||
| query_pb2 as ibc_connection_query, | ||
| query_pb2_grpc as ibc_connection_query_grpc, | ||
| ) | ||
|
|
||
|
|
||
| class ConfigurableIBCConnectionQueryServicer(ibc_connection_query_grpc.QueryServicer): | ||
| def __init__(self): | ||
| super().__init__() | ||
| self.connection_responses = deque() | ||
| self.connections_responses = deque() | ||
| self.client_connections_responses = deque() | ||
| self.connection_client_state_responses = deque() | ||
| self.connection_consensus_state_responses = deque() | ||
| self.connection_params_responses = deque() | ||
|
|
||
| async def Connection(self, request: ibc_connection_query.QueryConnectionRequest, context=None, metadata=None): | ||
| return self.connection_responses.pop() | ||
|
|
||
| async def Connections(self, request: ibc_connection_query.QueryConnectionsRequest, context=None, metadata=None): | ||
| return self.connections_responses.pop() | ||
|
|
||
| async def ClientConnections( | ||
| self, request: ibc_connection_query.QueryClientConnectionsRequest, context=None, metadata=None | ||
| ): | ||
| return self.client_connections_responses.pop() | ||
|
|
||
| async def ConnectionClientState( | ||
| self, request: ibc_connection_query.QueryConnectionClientStateRequest, context=None, metadata=None | ||
| ): | ||
| return self.connection_client_state_responses.pop() | ||
|
|
||
| async def ConnectionConsensusState( | ||
| self, request: ibc_connection_query.QueryConnectionConsensusStateRequest, context=None, metadata=None | ||
| ): | ||
| return self.connection_consensus_state_responses.pop() | ||
|
|
||
| async def ConnectionParams( | ||
| self, request: ibc_connection_query.QueryConnectionParamsRequest, context=None, metadata=None | ||
| ): | ||
| return self.connection_params_responses.pop() |
There was a problem hiding this comment.
Ensure robust handling of empty deques in servicer methods.
The methods in ConfigurableIBCConnectionQueryServicer pop items from deques without checking if they are empty. This could lead to IndexError if a deque is empty. Consider adding checks or default values to handle these cases safely.
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: pypi/pyyaml@6.0.1 |
Solves CHAIN-83
Summary by CodeRabbit
New Features
async_clientwith new methods for improved IBC connection handling.Refactor
Documentation
CHANGELOG.mdto reflect recent changes and additions.Tests
IBCConnectionGrpcApiclass, ensuring robust functionality through simulated gRPC interactions.