Integrated the SmartContractFactory #361
Conversation
| def prepare_execute_transaction_data(function: str, arguments: List[Any]) -> TransactionPayload: | ||
| tx_data = function | ||
|
|
||
| for arg in arguments: | ||
| tx_data += f"@{_prepare_argument(arg)}" | ||
|
|
||
| return TransactionPayload.from_str(tx_data) | ||
|
|
||
|
|
||
| def prepare_args_for_factory(arguments: List[str]) -> List[Any]: | ||
| args: List[Any] = [] |
There was a problem hiding this comment.
unit tests for these functions?
There was a problem hiding this comment.
Added a unit test for prepare_args_for_factory
multiversx_sdk_cli/contracts.py
Outdated
| function: str, | ||
| arguments: List[Any], | ||
| value: int = 0, | ||
| caller: Optional[Address] = None |
There was a problem hiding this comment.
| caller: Optional[Address] = None | |
| caller: Optional[IAddress] = None |
?
There was a problem hiding this comment.
Replaced where possible
| class IConfig(Protocol): | ||
| chain_id: str | ||
| min_gas_limit: int |
There was a problem hiding this comment.
from what i've seen IConfig and ITokenComputer are defined also in core, i'm thinking if it might be better to reuse from core
| @@ -68,7 +68,7 @@ def prepare_transaction_data_for_stake(node_operator_address: Address, validator | |||
| if reward_address: | |||
| call_arguments.append(f"0x{reward_address.to_hex()}") | |||
|
|
|||
| data = SmartContract().prepare_execute_transaction_data("stake", call_arguments) | |||
| data = prepare_execute_transaction_data("stake", call_arguments) | |||
There was a problem hiding this comment.
I think in the next PR, it will use the factory for staking operations, correct?
There was a problem hiding this comment.
Not really. I think this is for legacy staking? This is within the mxpy validator command group while the delegation factory from sdk-core is used for mxpy staking-provider command group.
There was a problem hiding this comment.
I mean this is used for the staking smart contract
There was a problem hiding this comment.
Not for legacy delegation. For the "lower-level" / "core" staking: https://docs.multiversx.com/validators/staking/staking-smart-contract/#staking.
| @@ -11,3 +11,5 @@ | |||
|
|
|||
| DEFAULT_HRP = "erd" | |||
| ADDRESS_ZERO_BECH32 = "erd1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq6gq4hu" | |||
|
|
|||
| NUMBER_OF_SHARDS = 3 | |||
There was a problem hiding this comment.
I see it was set to 3 before, as well (hardcoded).
But it seems there were no issues when performing deploys on localnet (2 shards, by default)?
Indeed, the contract address computation algorithm does not depend on the number of shards:
https://github.com/multiversx/mx-sdk-py-core/blob/main/multiversx_sdk_core/address.py#L109
Thus, we have a slightly unfortunate design (specs) of the AddressComputer. It requires the number of shards as a parameter, but that isn't required in all circumstances.
Can stay as it is 👍
There was a problem hiding this comment.
Indeed the number_of_shards is not used to compute the contract address, added it to be more explicit, but it can be deleted since the default value is already 3. Let me know what you think.
| def __init__(self, config: IConfig, token_computer: ITokenComputer): | ||
| self._factory = SmartContractTransactionsFactory(config, token_computer) | ||
|
|
||
| def get_deploy_transaction(self, owner: Account, args: Any) -> Transaction: |
There was a problem hiding this comment.
Makes quite a few assumptions about the content of args. Separate parameters (e.g. gasLimit, bytecode and so on) would work as well.
Can also stay as it is (this PR), since, as far as I see, SmartContract is only used by cli_contracts, correct?
There was a problem hiding this comment.
Yes, it's only used by cli_contracts.py. I thought it's a little bit "cleaner" to use args since gasLimit, bytecode and so on are required arguments, and we also check for the nonce and chainID args.
Integrated the
SmartContractFactoryinto mxpy.