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.

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

Smart Contract deploy transactions parser #22

Merged
merged 3 commits into from
Apr 15, 2024
Merged

Smart Contract deploy transactions parser #22

merged 3 commits into from
Apr 15, 2024

Conversation

popenta
Copy link
Collaborator

@popenta popenta commented Apr 10, 2024

Implemented the smart contract deploy transactions parser based on the sdk-specs.

@popenta popenta self-assigned this Apr 10, 2024
@ssd04 ssd04 self-requested a review April 10, 2024 15:49
Comment on lines +9 to +10
class SmartContractTransactionsOutcomeParser:
def __init__(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

in specs there is alo parse_execute do we plan to implement it also?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we'll implement that in the future. We need the abi for that but we currently don't have it in sdk-py.

@andreibancioiu andreibancioiu self-requested a review April 15, 2024 06:40
cryptography==36.0.2
pycryptodomex==3.19.1
protobuf==3.20.2
cryptography==42.0.2
Copy link
Contributor

Choose a reason for hiding this comment

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

No breaking changes that affect us, correct?

Let's triple-check here:
https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst

We should also adjust this workflow to include macos-latest and macos-13-xlarge (the latter is for MacOS ARM64):
https://github.com/multiversx/mx-sdk-py-incubator/blob/main/.github/workflows/test.yml#L16

The workflow adjustment can be done in a separate PR, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will take a look, and will also adjust the workflow in the next PR.

@@ -111,6 +112,7 @@ def from_http_response(
result.gas_limit = response.get("gasLimit", 0)

data = response.get("data", "") or ""
result.function = response.get("function", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,16 @@
from dataclasses import dataclass
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice approach 👍 (*_types.py file for outcome parsers).

Comment on lines +135 to +148
assert actual_tx_outcome.logs.address == expected_tx_outcome.logs.address
assert actual_tx_outcome.logs.events[0].identifier == expected_tx_outcome.logs.events[0].identifier
assert actual_tx_outcome.logs.events[0].data_items == expected_tx_outcome.logs.events[0].data_items
assert actual_tx_outcome.logs.events[0].address == expected_tx_outcome.logs.events[0].address
assert actual_tx_outcome.logs.events[0].topics == expected_tx_outcome.logs.events[0].topics

assert actual_tx_outcome.transaction_results[0].sender == expected_tx_outcome.transaction_results[0].sender
assert actual_tx_outcome.transaction_results[0].receiver == expected_tx_outcome.transaction_results[0].receiver
assert actual_tx_outcome.transaction_results[0].data == expected_tx_outcome.transaction_results[0].data
assert actual_tx_outcome.transaction_results[0].logs.address == expected_tx_outcome.transaction_results[0].logs.address
assert actual_tx_outcome.transaction_results[0].logs.events[0].address == expected_tx_outcome.transaction_results[0].logs.events[0].address
assert actual_tx_outcome.transaction_results[0].logs.events[0].identifier == expected_tx_outcome.transaction_results[0].logs.events[0].identifier
assert actual_tx_outcome.transaction_results[0].logs.events[0].data_items == expected_tx_outcome.transaction_results[0].logs.events[0].data_items
assert actual_tx_outcome.transaction_results[0].logs.events[0].topics == expected_tx_outcome.transaction_results[0].logs.events[0].topics
Copy link
Contributor

Choose a reason for hiding this comment

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

No way to do a shorter assert? Deep equality etc.?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No "native" way as far as I know. Will think about doing a custom deep_equal() method to use in some tests.

owner_address_topic = event.topics[1] if event.topics[1] else b''
code_hash_topic = event.topics[2] if event.topics[2] else b''

contract_address = Address(contract_address_topic, DEFAULT_ADDRESS_HRP).to_bech32() if len(contract_address_topic) else ""
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future, we have to parametrize such outcome parsers to receive the hrp.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, I was thinking the same.

assert len(parsed.contracts) == 0
assert parsed.contracts == []

@pytest.mark.networkInteraction
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

assert parsed.contracts[0].address == "erd1qqqqqqqqqqqqqpgq29deu3uhcvuk7jhxd5cxrvh23xulkcewd8ssyf38ec"
assert parsed.contracts[0].owner_address == "erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th"

@pytest.mark.networkInteraction
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@popenta popenta merged commit 2c80888 into main Apr 15, 2024
4 checks passed
@popenta popenta deleted the parse-sc-deploy branch April 15, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants