feat: update consensus ABI to v0.5 IConsensusMain interface#64
feat: update consensus ABI to v0.5 IConsensusMain interface#64MuncleUscles wants to merge 4 commits intomainfrom
Conversation
- Replace consensus_main_abi.json with v0.5 IConsensusMain ABI - Add valid_until parameter to addTransaction encoder/decoder - Update actions.py to pass valid_until (default 0 = no expiry)
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe PR updates the consensus contract ABI with major event and function signature refactoring, adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/unit/smoke/test_testnet_smoke.py`:
- Around line 90-91: The TestEncoderDecoderRoundTrip class is incorrectly marked
with `@pytest.mark.testnet` even though it only runs local encode/decode logic;
remove the decorator so these tests run by default. Locate the
TestEncoderDecoderRoundTrip class definition and delete the `@pytest.mark.testnet`
line (or move any necessary markers to individual network-dependent tests),
ensuring the class and its methods remain unchanged so encoder/decoder coverage
runs in the standard pytest suite.
- Around line 94-131: Add assertions to verify the new valid_until field
round-trips through encode_add_transaction_data and decode_add_transaction_data:
in test_round_trip_call (and similarly test_round_trip_deploy) assert
decoded["valid_until"] == 0 for the default case, and add a new test (or extend)
that calls encode_add_transaction_data with valid_until=1234567890 then
decode_add_transaction_data and assert decoded["valid_until"] == 1234567890;
reference encode_add_transaction_data, decode_add_transaction_data,
test_round_trip_call, and test_round_trip_deploy to locate the places to add
these assertions.
🧹 Nitpick comments (2)
genlayer_py/contracts/actions.py (1)
242-263:valid_untilis plumbed internally but not exposed in the public API.The
_encode_add_transaction_datafunction now acceptsvalid_until, but neitherwrite_contract(Line 115) nordeploy_contract(Line 151) forward this parameter to callers. Users cannot set a non-zero expiry through the public API. If this is intentional for an incremental rollout, consider adding a TODO or tracking issue. Otherwise, propagatevalid_untilthroughwrite_contractanddeploy_contract.♻️ Example: expose valid_until in write_contract
def write_contract( self: GenLayerClient, address: Union[Address, ChecksumAddress], function_name: str, account: Optional[LocalAccount] = None, consensus_max_rotations: Optional[int] = None, value: int = 0, leader_only: bool = False, args: Optional[List[CalldataEncodable]] = None, kwargs: Optional[Dict[str, CalldataEncodable]] = None, sim_config: Optional[SimConfig] = None, + valid_until: int = 0, ): ... encoded_data = _encode_add_transaction_data( self=self, sender_account=sender_account, recipient=address, consensus_max_rotations=consensus_max_rotations, data=serialized_data, + valid_until=valid_until, )Apply the same pattern to
deploy_contract.tests/unit/smoke/test_testnet_smoke.py (1)
42-52: Consider using a fixture to avoid repeatedWeb3instantiation.Each test in
TestTestnetConnectivitycreates a freshWeb3(Web3.HTTPProvider(...))instance. A class-scoped fixture would reduce duplication and connection overhead.♻️ Proposed refactor
`@pytest.mark.testnet` class TestTestnetConnectivity: """Verify RPC connectivity to testnet. Run with: pytest -m testnet""" + `@pytest.fixture`(autouse=True) + def setup(self): + self.w3 = Web3(Web3.HTTPProvider(TESTNET_JSON_RPC_URL)) + def test_rpc_connects(self): - w3 = Web3(Web3.HTTPProvider(TESTNET_JSON_RPC_URL)) - assert w3.is_connected() + assert self.w3.is_connected() def test_chain_id_matches(self): - w3 = Web3(Web3.HTTPProvider(TESTNET_JSON_RPC_URL)) - assert w3.eth.chain_id == 4221 + assert self.w3.eth.chain_id == 4221 def test_block_number_positive(self): - w3 = Web3(Web3.HTTPProvider(TESTNET_JSON_RPC_URL)) - assert w3.eth.block_number > 0 + assert self.w3.eth.block_number > 0
| @pytest.mark.testnet | ||
| class TestEncoderDecoderRoundTrip: |
There was a problem hiding this comment.
TestEncoderDecoderRoundTrip is marked @pytest.mark.testnet but doesn't use the network.
These tests perform purely local encode/decode operations and don't require testnet connectivity. Marking them testnet means they won't run in the default test suite (pytest without -m testnet), reducing coverage of the encoder/decoder logic in CI.
Remove the @pytest.mark.testnet marker so these tests run by default.
Proposed fix
-@pytest.mark.testnet
class TestEncoderDecoderRoundTrip:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.mark.testnet | |
| class TestEncoderDecoderRoundTrip: | |
| class TestEncoderDecoderRoundTrip: |
🤖 Prompt for AI Agents
In `@tests/unit/smoke/test_testnet_smoke.py` around lines 90 - 91, The
TestEncoderDecoderRoundTrip class is incorrectly marked with
`@pytest.mark.testnet` even though it only runs local encode/decode logic; remove
the decorator so these tests run by default. Locate the
TestEncoderDecoderRoundTrip class definition and delete the `@pytest.mark.testnet`
line (or move any necessary markers to individual network-dependent tests),
ensuring the class and its methods remain unchanged so encoder/decoder coverage
runs in the standard pytest suite.
| def test_round_trip_call(self): | ||
| from genlayer_py.consensus.consensus_main.encoder import ( | ||
| encode_add_transaction_data, | ||
| encode_tx_data_call, | ||
| ) | ||
| from genlayer_py.consensus.consensus_main.decoder import ( | ||
| decode_add_transaction_data, | ||
| ) | ||
|
|
||
| sender = "0xABcdEFABcdEFabcdEfAbCdefAbCdEFaBcDEFabCD" | ||
| recipient = "0x1111111111111111111111111111111111111111" | ||
| num_validators = 7 | ||
| max_rotations = 2 | ||
|
|
||
| tx_data = encode_tx_data_call( | ||
| function_name="my_method", | ||
| leader_only=True, | ||
| args=["hello"], | ||
| kwargs={"key": "value"}, | ||
| ) | ||
|
|
||
| encoded = encode_add_transaction_data( | ||
| sender_address=sender, | ||
| recipient_address=recipient, | ||
| num_of_initial_validators=num_validators, | ||
| max_rotations=max_rotations, | ||
| tx_data=tx_data.hex() if isinstance(tx_data, bytes) else tx_data, | ||
| ) | ||
|
|
||
| decoded = decode_add_transaction_data(encoded) | ||
| assert decoded["sender_address"].lower() == sender.lower() | ||
| assert decoded["recipient_address"].lower() == recipient.lower() | ||
| assert decoded["num_of_initial_validators"] == num_validators | ||
| assert decoded["max_rotations"] == max_rotations | ||
| assert decoded["tx_data"]["decoded"]["call_data"]["method"] == "my_method" | ||
| assert decoded["tx_data"]["decoded"]["leader_only"] is True | ||
| assert decoded["tx_data"]["decoded"]["type"] == "call" | ||
|
|
There was a problem hiding this comment.
Round-trip tests don't assert valid_until.
The valid_until field is the primary addition in this PR, but neither test_round_trip_call nor test_round_trip_deploy asserts its value after decoding. Add an assertion to verify the round-trip of valid_until (both default 0 and a non-zero value).
Proposed addition for test_round_trip_call
assert decoded["tx_data"]["decoded"]["leader_only"] is True
assert decoded["tx_data"]["decoded"]["type"] == "call"
+ assert decoded["valid_until"] == 0Consider also adding a test case with a non-zero valid_until to exercise the new parameter end-to-end:
def test_round_trip_call_with_valid_until(self):
# ... same setup ...
encoded = encode_add_transaction_data(
sender_address=sender,
recipient_address=recipient,
num_of_initial_validators=num_validators,
max_rotations=max_rotations,
tx_data=tx_data.hex() if isinstance(tx_data, bytes) else tx_data,
valid_until=1234567890,
)
decoded = decode_add_transaction_data(encoded)
assert decoded["valid_until"] == 1234567890📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_round_trip_call(self): | |
| from genlayer_py.consensus.consensus_main.encoder import ( | |
| encode_add_transaction_data, | |
| encode_tx_data_call, | |
| ) | |
| from genlayer_py.consensus.consensus_main.decoder import ( | |
| decode_add_transaction_data, | |
| ) | |
| sender = "0xABcdEFABcdEFabcdEfAbCdefAbCdEFaBcDEFabCD" | |
| recipient = "0x1111111111111111111111111111111111111111" | |
| num_validators = 7 | |
| max_rotations = 2 | |
| tx_data = encode_tx_data_call( | |
| function_name="my_method", | |
| leader_only=True, | |
| args=["hello"], | |
| kwargs={"key": "value"}, | |
| ) | |
| encoded = encode_add_transaction_data( | |
| sender_address=sender, | |
| recipient_address=recipient, | |
| num_of_initial_validators=num_validators, | |
| max_rotations=max_rotations, | |
| tx_data=tx_data.hex() if isinstance(tx_data, bytes) else tx_data, | |
| ) | |
| decoded = decode_add_transaction_data(encoded) | |
| assert decoded["sender_address"].lower() == sender.lower() | |
| assert decoded["recipient_address"].lower() == recipient.lower() | |
| assert decoded["num_of_initial_validators"] == num_validators | |
| assert decoded["max_rotations"] == max_rotations | |
| assert decoded["tx_data"]["decoded"]["call_data"]["method"] == "my_method" | |
| assert decoded["tx_data"]["decoded"]["leader_only"] is True | |
| assert decoded["tx_data"]["decoded"]["type"] == "call" | |
| def test_round_trip_call(self): | |
| from genlayer_py.consensus.consensus_main.encoder import ( | |
| encode_add_transaction_data, | |
| encode_tx_data_call, | |
| ) | |
| from genlayer_py.consensus.consensus_main.decoder import ( | |
| decode_add_transaction_data, | |
| ) | |
| sender = "0xABcdEFABcdEFabcdEfAbCdefAbCdEFaBcDEFabCD" | |
| recipient = "0x1111111111111111111111111111111111111111" | |
| num_validators = 7 | |
| max_rotations = 2 | |
| tx_data = encode_tx_data_call( | |
| function_name="my_method", | |
| leader_only=True, | |
| args=["hello"], | |
| kwargs={"key": "value"}, | |
| ) | |
| encoded = encode_add_transaction_data( | |
| sender_address=sender, | |
| recipient_address=recipient, | |
| num_of_initial_validators=num_validators, | |
| max_rotations=max_rotations, | |
| tx_data=tx_data.hex() if isinstance(tx_data, bytes) else tx_data, | |
| ) | |
| decoded = decode_add_transaction_data(encoded) | |
| assert decoded["sender_address"].lower() == sender.lower() | |
| assert decoded["recipient_address"].lower() == recipient.lower() | |
| assert decoded["num_of_initial_validators"] == num_validators | |
| assert decoded["max_rotations"] == max_rotations | |
| assert decoded["tx_data"]["decoded"]["call_data"]["method"] == "my_method" | |
| assert decoded["tx_data"]["decoded"]["leader_only"] is True | |
| assert decoded["tx_data"]["decoded"]["type"] == "call" | |
| assert decoded["valid_until"] == 0 |
🤖 Prompt for AI Agents
In `@tests/unit/smoke/test_testnet_smoke.py` around lines 94 - 131, Add assertions
to verify the new valid_until field round-trips through
encode_add_transaction_data and decode_add_transaction_data: in
test_round_trip_call (and similarly test_round_trip_deploy) assert
decoded["valid_until"] == 0 for the default case, and add a new test (or extend)
that calls encode_add_transaction_data with valid_until=1234567890 then
decode_add_transaction_data and assert decoded["valid_until"] == 1234567890;
reference encode_add_transaction_data, decode_add_transaction_data,
test_round_trip_call, and test_round_trip_deploy to locate the places to add
these assertions.
1 participant
Summary
valid_untilparameter toaddTransactionencoder (default 0 = no expiry)valid_untilto decoder output for round-trip compatibilityvalid_untilthrough the encoding pipelineTest plan
addTransactionsignature matches v0.5:addTransaction(address,address,uint256,uint256,bytes,uint256)valid_until=0Summary by CodeRabbit
New Features
valid_untilparameter for encoding and decoding transaction data.Chores
Tests