Conversation
Merge main in feat/next
Remove relayed v1 and v2 & other deprecated code
Gas limit estimator
v2: Bump version & generate docs
Coverage report
Click to see where and how coverage changedThe report is truncated to 25 files out of 77. To see the full report, please visit the workflow summary page. This report was generated by python-coverage-comment-action |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the SDK to version 2.0.0 and introduces significant architectural changes to transaction factories and controllers by implementing a new gas limit estimation system and removing deprecated relayed transaction functionality.
- Migrated from TransactionBuilder pattern to BaseFactory with gas limit estimation support
- Introduced GasLimitEstimator for automatic gas limit calculation with network provider integration
- Removed deprecated relayed transactions V1/V2 functionality and components
Reviewed Changes
Copilot reviewed 63 out of 64 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bump to 2.0.0 |
| multiversx_sdk/validators/validators_transactions_factory.py | Refactored to inherit from BaseFactory, replaced TransactionBuilder usage |
| multiversx_sdk/transfers/transfer_transactions_factory.py | Converted to BaseFactory pattern with gas limit estimator support |
| multiversx_sdk/token_management/token_management_transactions_factory.py | Updated factory to use BaseFactory and new gas estimation system |
| multiversx_sdk/smart_contracts/smart_contract_transactions_factory.py | Migrated to BaseFactory, made gas_limit optional with estimator fallback |
| multiversx_sdk/gas_estimator/ | New module providing GasLimitEstimator class for automatic gas calculation |
| multiversx_sdk/core/base_factory.py | New base class for all transaction factories with gas estimation logic |
| multiversx_sdk/entrypoints/entrypoints.py | Added gas limit estimator support to all entrypoint factory methods |
| multiversx_sdk/builders/transaction_builder.py | Removed deprecated TransactionBuilder class |
| multiversx_sdk/relayed/ | Completely removed deprecated relayed transaction components |
Comments suppressed due to low confidence (1)
multiversx_sdk/account_management/account_controller.py:13
- Parameter name 'gasLimitEstimator' should follow snake_case convention to match other parameters in the codebase. It should be 'gas_limit_estimator'.
def __init__(self, chain_id: str, gasLimitEstimator: Optional[IGasLimitEstimator] = None) -> None:
| sender = Address.new_from_bech32("erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th") | ||
| contract_address = Address.new_from_bech32("erd1qqqqqqqqqqqqqpgqhy6nl6zq07rnzry8uyh6rtyq0uzgtk3e69fqgtz9l4") | ||
| new_owner = Address.from_bech32("erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx") | ||
| new_owner = Address.new_from_bech32("erd1spyavw0956vq68xj8y4tenjpq2wd5a9p2c6j8gsz7ztyrnpxrruqzu66jx") |
There was a problem hiding this comment.
The method call was changed from 'Address.from_bech32' to 'Address.new_from_bech32', but this appears to be updating deprecated method usage which is good. However, this change suggests a breaking API change that should be documented.
| if tx_copy.guardian and not tx_copy.guardian_signature: | ||
| tx_copy.guardian_signature = bytes([0]) * 64 | ||
|
|
||
| if tx_copy.relayer_signature and not tx_copy.relayer_signature: |
There was a problem hiding this comment.
The condition 'tx_copy.relayer_signature and not tx_copy.relayer_signature' is logically impossible and will never be true. This should likely be 'if tx_copy.relayer and not tx_copy.relayer_signature:' to check if relayer exists but signature is missing.
| if tx_copy.relayer_signature and not tx_copy.relayer_signature: | |
| if tx_copy.relayer and not tx_copy.relayer_signature: |
| if tx_copy.guardian and not tx_copy.guardian_signature: | ||
| tx_copy.guardian_signature = bytes([0]) * 64 | ||
|
|
||
| if tx_copy.relayer_signature and not tx_copy.relayer_signature: |
There was a problem hiding this comment.
The condition 'tx_copy.relayer_signature and not tx_copy.relayer_signature' is logically impossible and will never be true. This should likely be 'if tx_copy.relayer and not tx_copy.relayer_signature:' to check if relayer exists but signature is missing.
| if tx_copy.relayer_signature and not tx_copy.relayer_signature: | |
| if tx_copy.relayer and not tx_copy.relayer_signature: |
| gas_limit=60_000_000, | ||
| native_token_amount=50000000000000000, |
There was a problem hiding this comment.
The parameter order has been changed - 'gas_limit' moved after 'contract_to_copy'. This is a breaking change in the API that could affect existing code and should be carefully documented.
| gas_limit=60_000_000, | |
| native_token_amount=50000000000000000, | |
| native_token_amount=50000000000000000, | |
| gas_limit=60_000_000, |
Fix patching transaction for /cost endpoint
No description provided.