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.

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

Add missing field to ExecutionResources #1301

Merged
merged 7 commits into from
Feb 23, 2024
Merged

Add missing field to ExecutionResources #1301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4779533
Add missing field to ExecutionResources
tkumor3 Feb 22, 2024
759902d
linter
tkumor3 Feb 22, 2024
fe9ed31
add test
tkumor3 Feb 22, 2024
fa48097
rename test
tkumor3 Feb 22, 2024
7dd5972
Merge branch 'development' into tomaszkumor/execution_resources_hotfix
ddoktorski Feb 23, 2024
4e073b1
Assert other execution resources fields in test
ddoktorski Feb 23, 2024
abfa894
Use integers instead of felts for execution resources fields
ddoktorski Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions starknet_py/net/client_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ class ExecutionResources:
bitwise_builtin_applications: Optional[int] = None
keccak_builtin_applications: Optional[int] = None
memory_holes: Optional[int] = None
segment_arena_builtin: Optional[int] = None


# TODO (#1219): split into PendingTransactionReceipt and TransactionReceipt
Expand Down
21 changes: 12 additions & 9 deletions starknet_py/net/schemas/rpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,29 +116,32 @@ def make_dataclass(self, data, **kwargs) -> L2toL1Message:


class ExecutionResourcesSchema(Schema):
steps = Felt(data_key="steps", required=True)
range_check_builtin_applications = Felt(
steps = fields.Integer(data_key="steps", required=True)
range_check_builtin_applications = fields.Integer(
data_key="range_check_builtin_applications", load_default=None
)
pedersen_builtin_applications = Felt(
pedersen_builtin_applications = fields.Integer(
data_key="pedersen_builtin_applications", load_default=None
)
poseidon_builtin_applications = Felt(
poseidon_builtin_applications = fields.Integer(
data_key="poseidon_builtin_applications", load_default=None
)
ec_op_builtin_applications = Felt(
ec_op_builtin_applications = fields.Integer(
data_key="ec_op_builtin_applications", load_default=None
)
ecdsa_builtin_applications = Felt(
ecdsa_builtin_applications = fields.Integer(
data_key="ecdsa_builtin_applications", load_default=None
)
bitwise_builtin_applications = Felt(
bitwise_builtin_applications = fields.Integer(
data_key="bitwise_builtin_applications", load_default=None
)
keccak_builtin_applications = Felt(
keccak_builtin_applications = fields.Integer(
data_key="keccak_builtin_applications", load_default=None
)
memory_holes = Felt(data_key="memory_holes", load_default=None)
segment_arena_builtin = fields.Integer(
data_key="segment_arena_builtin", load_default=None
)
memory_holes = fields.Integer(data_key="memory_holes", load_default=None)

@post_load
def make_dataclass(self, data, **kwargs) -> ExecutionResources:
Expand Down
16 changes: 16 additions & 0 deletions starknet_py/tests/e2e/tests_on_networks/client_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,3 +425,19 @@ async def test_get_events_sepolia_testnet(client_sepolia_testnet):
assert isinstance(events_chunk, EventsChunk)
assert len(events_chunk.events) == 10
assert events_chunk.continuation_token is not None


@pytest.mark.asyncio
async def test_get_tx_receipt_with_execution_resources(client_sepolia_integration):
receipt = await client_sepolia_integration.get_transaction_receipt(
tx_hash=0x077E84B7C0C4CC88B778EEAEF32B7CED4500FE4AAEE62FD2F849B7DD90A87826
)

assert receipt.execution_resources is not None
assert receipt.execution_resources.steps is not None
assert receipt.execution_resources.segment_arena_builtin is not None
assert receipt.execution_resources.bitwise_builtin_applications is not None
assert receipt.execution_resources.ec_op_builtin_applications is not None
assert receipt.execution_resources.memory_holes is not None
assert receipt.execution_resources.pedersen_builtin_applications is not None
assert receipt.execution_resources.range_check_builtin_applications is not None
Comment on lines +436 to +443
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: since this is a hardcoded transaction, it should be possible to just assert specific values that we expect (and not just asserting that it is not None), which would give us another layer of testing (I mean serialization).

if you agree, I'd suggest we open an issue and deal with it in a separate PR, at later date.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In an ideal scenario, we should probably have unit tests for out RPC schemas 😅 However the check that you suggest can definitely improve this test. Let's discuss it further in #1219

Loading