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-1165] Add --valid-at/--expires-at to all CLI transaction endpoints (mostly hidden) #18501

Merged
merged 6 commits into from
Sep 5, 2024

Conversation

Quexington
Copy link
Contributor

@Quexington Quexington commented Aug 20, 2024

This PR introduces the option to add expiration/delayed activation to any transaction generated from the CLI. This is functionality that is available for offers at the RPC/GUI level and this PR seeks to make all transaction endpoints unified in this respect.

However, the existing wallet UX has some shortcomings that make adding this functionality difficult. For example, we do not internally mark transactions as expired when they expire, nor do we necessarily resubmit ones that are not yet valid. Without some proper though into these shortcomings, I'm hesitant to make this fully available and supported.

However, all transaction endpoints do currently have the ability to behave like offers (export the transaction to computer and not push it) so if a user wants to use it like that, I don't really see a reason why they shouldn't be allowed.

As a compromise between the tension between usefulness and UX concerns, I've decided to make the options hidden for every command except make_offer. The options can still be used, but a user examining the help won't be able to see that they are, in fact, valid options. This means we can not really fully support the functionality, but still make it useful to adventurous users who have applications in mind.

@Quexington Quexington added the Added Required label for PR that categorizes merge commit message as "Added" for changelog label Aug 20, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Aug 20, 2024
@Chia-Network Chia-Network deleted a comment from coveralls-official bot Aug 20, 2024
@Quexington Quexington force-pushed the quex.add_timelock_args_to_cli branch from 3d2b1cb to 69a917a Compare August 20, 2024 20:16
@Quexington Quexington changed the title [CHIA-1165] Add --valid-at/--expires-at to all CLI transaction endpoints [CHIA-1165] Add --valid-at/--expires-at to all CLI transaction endpoints (mostly hidden) Aug 20, 2024
@Quexington Quexington marked this pull request as ready for review August 20, 2024 20:23
@Quexington Quexington requested a review from a team as a code owner August 20, 2024 20:23
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Aug 28, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Aug 29, 2024
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Aug 29, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Aug 29, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Aug 29, 2024
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Aug 30, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Sep 3, 2024
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Sep 3, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Sep 3, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Sep 3, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Sep 4, 2024
@Quexington Quexington closed this Sep 4, 2024
@Quexington Quexington reopened this Sep 4, 2024
Copy link
Contributor

github-actions bot commented Sep 5, 2024

File Coverage Missing Lines
chia/cmds/wallet_funcs.py 80.0% lines 1376
Total Missing Coverage
119 lines Unknown 99%

@markelrod
Copy link
Contributor

coverage exception ok

@Quexington Quexington added ready_to_merge Submitter and reviewers think this is ready and removed coverage-diff labels Sep 5, 2024
@pmaslana pmaslana merged commit 8e3a42c into main Sep 5, 2024
739 of 741 checks passed
@pmaslana pmaslana deleted the quex.add_timelock_args_to_cli branch September 5, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added Required label for PR that categorizes merge commit message as "Added" for changelog ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants