[CHIA-1348] Port chia wallet coin commands to @chia_command framework
#18641
Conversation
Quexington
added
Changed
Quexington
changed the title
chia wallet coin commands to @chia_command frameworkchia wallet coin commands to @chia_command framework
github-actions
bot
removed
the
merge_conflict
github-actions
bot
added
the
merge_conflict
github-actions
bot
removed
the
merge_conflict
github-actions
bot
added
coverage-diff
merge_conflict
github-actions
bot
removed
the
merge_conflict
|
| number_of_coins=100, | ||
| amount_per_coin=CliAmount(amount=uint64(100), mojos=True), | ||
| target_coin_id=target_coin.name(), | ||
| fee=1_000_000_000_000, # 1 XCH |
There was a problem hiding this comment.
As I read it this is still appropriate, just that the hinting is lost with this general dynamic handling.
| fee=1_000_000_000_000, # 1 XCH | |
| fee=uint64(1_000_000_000_000), # 1 XCH |
There was a problem hiding this comment.
In this scenario aren't we really just throwing the uint64 on it to appease mypy though? That was my reading, that we now have the privledge of doing away with this.
There was a problem hiding this comment.
TransactionEndpoint hints the fee as uint64, so shouldn't it be one? Yes, in this case we have made it so mypy just throws its hands up in despair :] because it can't tell what anything is or should be, but that doesn't seem like a good thing to take advantage of unnecessarily. or maybe i'm misreading here.
| ).to_tx_config(mojo_per_unit, config, fingerprint) | ||
|
|
||
|
|
||
| _DECORATOR_APPLIED = "_DECORATOR_APPLIED" |
There was a problem hiding this comment.
for stuff like this i often add get/set/is helper functions to spread around instead of the actual getattar()/setattr()/hasattr() calls. and make it really unlikely to collide with anything else.
(providing a suggestion here, but it would at least require this be moved so would have to be applied locally)
| _DECORATOR_APPLIED = "_DECORATOR_APPLIED" | |
| _TRANSACTION_ENDPOINT_RUNNER_DECORATOR_APPLIED_ATTRIBUTE_NAME = f"_{__name__.replace('.', '_')}_{transaction_endpoint_runner.__qualname__}_applied" |
| STANDARD_TX_ENDPOINT_ARGS: dict[str, Any] = TransactionEndpoint( | ||
| rpc_info=NeedsWalletRPC(client_info=None, wallet_rpc_port=None, fingerprint=None), | ||
| tx_config_loader=NeedsTXConfig( | ||
| min_coin_amount=cli_amount_none, | ||
| max_coin_amount=cli_amount_none, | ||
| coins_to_exclude=(), | ||
| amounts_to_exclude=(), | ||
| reuse=None, | ||
| ), | ||
| transaction_writer=TransactionsOut(transaction_file_out=None), | ||
| fee=uint64(0), | ||
| push=True, | ||
| valid_at=None, | ||
| expires_at=None, | ||
| ).__dict__ |
There was a problem hiding this comment.
The .__dict__ isn't quite as related to the constructor as the result of dataclasses.asdict() is. .asdict() itself isn't even fully correct depending if you use leading _'s, but it's better.
(whitespace-less review will likely be beneficial)
This PR takes a step forward in porting existing wallet commands to the
@chia_commandframework currently in use for the signer commands. The primary benefit of this framework is that we can start writing integration tests with the command line interface at the top instead of the RPC so we can actually test the entire stack that a user utilizes when interacting with our application. It also comes with strong typing for CLI parameters and better code unity (right now the click options and the actual execution code live in separate places).The coin commands were chosen due to the smaller number of them and the recent attention that has been paid to their tests making it easy to port the commands right on top of those recent changes.