Facade implementation #88

popenta · 2024-08-13T10:46:27Z

Implemented the facade and controllers based on the specs.

github-actions · 2024-08-13T10:47:18Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
multiversx_sdk
__init__.py
multiversx_sdk/controllers
__init__.py
account_controller.py					13-14, 20-28, 35-44, 49-56, 61-68
delegation_controller.py					25-28, 36-46, 50-53, 56-57, 65-75, 82-91, 98-107, 114-123, 130-139, 147-157, 164-173, 180-189, 195-203, 209-217, 223-231, 237-245, 254-265, 272-281, 287-295, 301-309, 316-325, 331-339
interfaces.py					9, 12
network_provider_wrapper.py					14
relayed_controller.py					22-30, 37-46
smart_contract_controller.py					26, 29, 32, 35, 38, 48, 53, 58, 61, 64, 67, 132-148, 175, 178
token_management_controller.py					30-34, 49-66, 69-70, 73-74, 88-104, 107-108, 111-112, 126-142, 145-146, 149-150, 165-182, 185-186, 189-190, 199-210, 213-214, 217-218, 224-232, 235-236, 239-240, 246-254, 257-258, 261-262, 272-284, 287-288, 291-292, 303-316, 319-320, 323-324, 336-350, 353-354, 357-358, 370-384, 387-388, 391-392, 398-406, 409-410, 413-414, 420-428, 431-432, 435-436, 443-452, 455-456, 459-460, 467-476, 479-480, 483-484, 491-500, 503-504, 507-508, 515-524, 527-528, 531-532, 539-548, 551-552, 555-556, 564-574, 577-578, 581-582, 590-600, 603-604, 607-608, 616-626, 629-630, 633-634
transfers_controller.py					23-33, 40-49
multiversx_sdk/core
__init__.py
multiversx_sdk/core/transactions_outcome_parsers
__init__.py
multiversx_sdk/facades
__init__.py
account.py					26-28, 35-37
config.py
entrypoints.py					33, 36, 39, 42, 51, 55, 61-62, 65-68, 75-76, 79-85, 103, 114, 117, 120, 129, 137-141, 155-159, 168
errors.py					3, 8
multiversx_sdk/network_providers
api_network_provider.py
Project Total

_{This report was generated by python-coverage-comment-action}

andreibancioiu · 2024-08-16T06:56:41Z

multiversx_sdk/core/controllers/account_controller.py

+class INetworkConfig(Protocol):
+    chain_id: str
+
+
+class INetworkProvider(Protocol):
+    def get_network_config(self) -> INetworkConfig:
+        ...


In this context, I think we can move these interfaces (defined a few times) directly in core/controllers/interfaces.py.

Later edit: by receiving the chain_id directly in controllers (see below), these won't be needed.

Removed it.

andreibancioiu · 2024-08-16T06:58:54Z

multiversx_sdk/core/controllers/account_controller.py

+
+    def _ensure_factory_is_initialized(self):
+        if self.factory is None:
+            self.chain_id = self.provider.get_network_config().chain_id


Let's receive chain_id directly in constructor, maybe? Here, and in other controllers. Then, I think we won't need the network provider in the controller anymore.

Indeed, now passing chain_id in the constructor of the controllers.

andreibancioiu · 2024-08-16T07:00:05Z

multiversx_sdk/core/controllers/account_controller.py

+                                                service_id: str) -> Transaction:
+        self._ensure_factory_is_initialized()
+
+        transaction = self.factory.create_transaction_for_setting_guardian(  # type: ignore


Is type: ignore needed?

not needed anymore

andreibancioiu · 2024-08-16T07:02:37Z

multiversx_sdk/facades/config.py

+
+@dataclass
+class TestnetConfig:
+    api = "https://testnet-api.multiversx.com"


Can be network_provider_url.

andreibancioiu · 2024-08-16T07:10:24Z

multiversx_sdk/facades/config.py

+@dataclass
+class TestnetConfig:
+    api = "https://testnet-api.multiversx.com"
+    kind = "api"


Can be network_provider_kind (I think).

andreibancioiu · 2024-08-16T09:59:21Z

multiversx_sdk/facades/errors.py

@@ -0,0 +1,13 @@
+class UnsuportedFileTypeError(Exception):


Is it used? Can be dropped if not.

andreibancioiu · 2024-08-16T10:01:53Z

multiversx_sdk/core/controllers/smart_contract_controller.py

+    def get_network_config(self) -> INetworkConfig:
+        ...
+
+    def query_contract(self, query: IQuery) -> IQueryResponse:


In feat/next, we should drop the old types on queries (cleanup), and only use the new ones:

https://github.com/multiversx/mx-sdk-py/blob/main/multiversx_sdk/core/smart_contract_query.py

If possible, can adjust the network provider in this PR, otherwise, should be done in a future PR.

Will do it in a future PR.

andreibancioiu · 2024-08-16T10:04:26Z

multiversx_sdk/facades/entrypoints.py

@@ -0,0 +1,163 @@
+


A few short unit tests would be nice. Can be based on the examples from the specs (but not necessarily):

https://github.com/multiversx/mx-sdk-specs/blob/main/facades/entrypoints.md#examples

added a few.

andreibancioiu · 2024-08-16T10:06:16Z

multiversx_sdk/core/controllers/smart_contract_controller.py

+        )
+
+        transaction.nonce = nonce
+        transaction.signature = sender.sign(self.tx_computer.compute_bytes_for_signing(transaction))


Maybe we can have a base class for controllers, which holds common functionality - e.g a private function to sign transactions (just an idea)?

Will keep it in mind as it could be a good idea, but as of now I don't think it is necessary.

andreibancioiu · 2024-08-16T10:06:43Z

multiversx_sdk/core/controllers/smart_contract_controller.py

+        pass
+
+    def await_completed_execute(self, tx_hash: str):
+        pass


Not implemented, perhaps throw an exception? Here & above?

danielailie · 2024-09-05T10:39:07Z

multiversx_sdk/controllers/delegation_controller.py

+from multiversx_sdk.converters.transactions_converter import \
+    TransactionsConverter
+from multiversx_sdk.core.interfaces import IAddress, IValidatorPublicKey
+from multiversx_sdk.core.transaction import Transaction


on imports I would've group imports based on their namespace, for example
from multiversx_sdk.core.transactions_outcome_parsers import DelegationTransactionsOutcomeParser, CreateNewDelegationContractOutcome

Did some of them, will probably do the same in all the files in the future PRs.

andreibancioiu · 2024-09-06T14:01:15Z

multiversx_sdk/controllers/delegation_controller.py

        self.provider = network_provider
+        self.factory = DelegationTransactionsFactory(TransactionsFactoryConfig(chain_id))


self.provider was removed from the other controllers. Is it required in this one?

It is not. Removed it from all controllers.

andreibancioiu · 2024-09-06T14:01:36Z

multiversx_sdk/controllers/delegation_controller.py

-        self.chain_id: Union[str, None] = None
-        self.factory: Union[DelegationTransactionsFactory, None] = None
-        self.parser = DelegationTransactionsOutcomeParser()
+    def __init__(self, chain_id: str, network_provider: Any) -> None:


Is network_provider: Any necessary?

As discussed, we'll use a temporary interface until the upcoming change for the network providers.

andreibancioiu · 2024-09-06T14:03:25Z

multiversx_sdk/controllers/smart_contract_controller.py

-    def parse_execute(self, transaction_on_network: TransactionOnNetwork, function: Optional[str] = None):
-        pass
+    def parse_execute(self, transaction_on_network: TransactionOnNetwork, function: Optional[str] = None) -> List[Any]:
+        raise NotImplementedError("This method is not yet implemented")


True. We'll see how to use this in the future:

multiversx/mx-chain-proxy-go#446

andreibancioiu · 2024-09-06T14:04:46Z

multiversx_sdk/facades/entrypoints.py

        tx_computer = TransactionComputer()
-        transaction.signature = signer.sign(tx_computer.compute_bytes_for_signing(transaction))
+        transaction.signature = account.sign(tx_computer.compute_bytes_for_signing(transaction))


Type is still UserSigner, is this intended?

changed to Account.

andreibancioiu · 2024-09-06T14:07:49Z

multiversx_sdk/facades/entrypoints_test.py

+        abi = Abi.load(testutils / "testdata" / "adder.abi.json")
+        sender = Account.new_from_pem(self.alice_pem)
+        sender.nonce = self.entrypoint.recall_account_nonce(sender.address)
+
+        controller = self.entrypoint.create_smart_contract_controller(abi)
+        bytecode = (testutils / "testdata" / "adder.wasm").read_bytes()
+        transaction = controller.create_transaction_for_deploy(
+            sender=sender,
+            nonce=sender.get_nonce_then_increment(),
+            bytecode=bytecode,
+            gas_limit=10_000_000,
+            arguments=[0]
+        )
+
+        tx_hash = self.entrypoint.send_transaction(transaction)
+        outcome = controller.await_completed_deploy(tx_hash)
+


andreibancioiu · 2024-09-06T14:10:15Z

multiversx_sdk/facades/entrypoints_test.py

+        retries = 10
+        while retries:
+            time.sleep(0.5)
+            try:
+                self.entrypoint.await_completed_transaction(tx_hash)
+                break
+            except:
+                retries -= 1


Not sure why this is needed.

Perhaps due a possible 404 transaction not found when invoking await_completed_transaction immediately after send_transaction? If so, let's add a trivial sleep after broadcasting the transaction.

We should make the transaction completion awaiting a bit more robust (in the future, separate PR).

Indeed, it was due to a 404 error. Added a simple time.sleep(1). Will make the transaction awaiting more robust in the future.

popenta added 11 commits June 4, 2024 16:28

first facade sketch

3960a3e

Merge branch 'main' into facade

c1461b2

continue sketch

af71228

Merge branch 'feat/next' into facade

f4c6b66

update to current specs

341aca5

update facade to current specs

7044659

Merge branch 'main' into facade

edc7f3f

use abi inside smart contracts controller

2019385

Merge branch 'main' into facade

0b4aaba

update facade to specs

e0b99bf

Merge branch 'feat/next' into facade

1a2ae53

popenta self-assigned this Aug 13, 2024

Merge branch 'feat/next' into facade

ef831d9

andreibancioiu reviewed Aug 16, 2024

View reviewed changes

popenta added 4 commits August 16, 2024 17:20

first part of fixes after review

eff977c

more fixes after review

07a8fff

extract controllers from core

358e0bb

small fixes & unit tests

9a81fd3

danielailie reviewed Sep 5, 2024

View reviewed changes

danielailie previously approved these changes Sep 5, 2024

View reviewed changes

popenta added 2 commits September 6, 2024 10:58

Merge branch 'feat/next' into facade

bae49e0

adjust exports and small fixes

0501acf

popenta dismissed danielailie’s stale review via 0501acf September 6, 2024 09:16

andreibancioiu reviewed Sep 6, 2024

View reviewed changes

popenta added 2 commits September 9, 2024 11:24

fixes after review

8e0771f

fix failing test

200d60b

andreibancioiu approved these changes Sep 9, 2024

View reviewed changes

danielailie approved these changes Sep 9, 2024

View reviewed changes

popenta merged commit 8552aaf into feat/next Sep 9, 2024
7 checks passed

popenta deleted the facade branch September 9, 2024 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Facade implementation #88

Facade implementation #88

popenta commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

andreibancioiu Aug 16, 2024

popenta Aug 16, 2024

andreibancioiu Aug 16, 2024

popenta Aug 16, 2024

andreibancioiu Aug 16, 2024

popenta Aug 16, 2024

andreibancioiu Aug 16, 2024

popenta Aug 16, 2024

andreibancioiu Aug 16, 2024

popenta Aug 16, 2024

andreibancioiu Aug 16, 2024

popenta Aug 16, 2024

andreibancioiu Aug 16, 2024

popenta Aug 19, 2024

andreibancioiu Aug 16, 2024

popenta Aug 19, 2024

andreibancioiu Aug 16, 2024

popenta Aug 19, 2024

andreibancioiu Aug 16, 2024

popenta Aug 19, 2024

danielailie Sep 5, 2024

popenta Sep 6, 2024

andreibancioiu Sep 6, 2024

popenta Sep 9, 2024 •

edited

Loading

andreibancioiu Sep 6, 2024

popenta Sep 9, 2024

andreibancioiu Sep 6, 2024

andreibancioiu Sep 6, 2024

popenta Sep 9, 2024

andreibancioiu Sep 6, 2024

andreibancioiu Sep 6, 2024

popenta Sep 9, 2024

		self.provider = network_provider
		self.factory = DelegationTransactionsFactory(TransactionsFactoryConfig(chain_id))

Facade implementation #88

Facade implementation #88

Conversation

popenta commented Aug 13, 2024 • edited Loading

github-actions bot commented Aug 13, 2024 • edited Loading

Coverage report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

popenta Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

popenta commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

popenta Sep 9, 2024 •

edited

Loading