Skip to content
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-1306] Tweak TransactionEndpointRequest to require TX args #18601

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

Quexington
Copy link
Contributor

This PR bans the previous standard way to serialize TransactionEndpointRequests in favor of one that requires attention to the common args that all of them should have. This is utilized instead of just including those onto the dataclass because streamable/clvm streamable/tx config makes this a hard ask. This should prevent any attempts on sending the request to the RPC without adequately taking into account the arguments that a user should have the chance to specify.

@Quexington Quexington added Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes labels Sep 20, 2024
Copy link
Contributor

File Coverage Missing Lines
chia/rpc/wallet_request_types.py 88.9% lines 288
Total Missing Coverage
13 lines Unknown 92%

@Quexington Quexington marked this pull request as ready for review September 23, 2024 14:34
@Quexington Quexington requested a review from a team as a code owner September 23, 2024 14:34
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also chatting with quex to get myself ramped up in this area.

chia/rpc/wallet_request_types.py Show resolved Hide resolved
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a small change within a bunch of conflicting needs. Would be great to do it better but I'll take this for now.

@pmaslana pmaslana merged commit 0d8d38d into main Sep 25, 2024
373 of 374 checks passed
@pmaslana pmaslana deleted the quex.enhance_ter branch September 25, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants