-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[CHIA-1308] Port wallet node RPC endpoints to @marshal
decorator
#18651
base: main
Are you sure you want to change the base?
Conversation
420fb7f
to
cc17b41
Compare
646912e
to
5dc8efd
Compare
@@ -292,7 +298,10 @@ class PushTransactionsCMD: | |||
|
|||
async def run(self) -> None: | |||
async with self.rpc_info.wallet_rpc() as wallet_rpc: | |||
await wallet_rpc.client.push_transactions(self.txs_in.transaction_bundle.txs) | |||
# TODO: no default here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm? not clear if this is for later or now or what the needed action is
@@ -694,20 +693,23 @@ async def push_transactions( | |||
|
|||
interface.side_effects.transactions.extend(inner_action_scope.side_effects.transactions) | |||
|
|||
return {} | |||
return PushTransactionsResponse([], []) # tx_endpoint takes care of this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems fishy to need this if it's pointless?
resp = await client.fetch( | ||
"push_transactions", {"transactions": [tx.to_json_dict_convenience(wallet_node.config)], "fee": 10} | ||
) | ||
assert resp["success"] | ||
resp = await client.fetch("push_transactions", {"transactions": [tx.to_json_dict()], "fee": 10}) | ||
resp = await client.fetch("push_transactions", {"transactions": [bytes(tx).hex()], "fee": 10}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we changing the api? or did tx.to_json_dict()
create a string before?
@@ -513,8 +518,8 @@ async def test_get_timestamp_for_height(wallet_rpc_environment: WalletRpcTestEnv | |||
|
|||
await generate_funds(full_node_api, env.wallet_1) | |||
|
|||
# This tests that the client returns a uint64, rather than raising or returning something unexpected | |||
uint64(await client.get_timestamp_for_height(uint32(1))) | |||
# This tests that the client returns a sucessfully, rather than raising or returning something unexpected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# This tests that the client returns a sucessfully, rather than raising or returning something unexpected | |
# This tests that the client returns successfully, rather than raising or returning something unexpected |
This is another PR in the same vein as #18593 .