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

Clean up command line flags #494

Open
fhenneke opened this issue Jan 16, 2025 · 1 comment
Open

Clean up command line flags #494

fhenneke opened this issue Jan 16, 2025 · 1 comment
Labels
E:9.1 Streamline mainnet payouts process See https://github.com/cowprotocol/pm/issues/80 for details

Comments

@fhenneke
Copy link
Collaborator

fhenneke commented Jan 16, 2025

We currently support lots of command line flags. Some of them are a bit redundant (see this comment on --post-tx and --dry-run). Some have an overlap with configuration parameters (--ignore-slippage vs config.buffer_accounting_config.include_slippage). After merging #492 we have another one (--send-to-slack).

Cleaning up the those flags and simplifying the logic where they are used would be useful.

@fhenneke fhenneke added the E:9.1 Streamline mainnet payouts process See https://github.com/cowprotocol/pm/issues/80 for details label Jan 16, 2025
@fhenneke
Copy link
Collaborator Author

More context on flags for output of the script:

Without this PR, we have the flags --post-tx and --dry-run.

  • Independent of those flags, transfers (and overdrafts) for payouts are computed.
  • The --post-tx flag triggers a code path where auto_propose is called. That function requires the signing key as it actually sets up a signed transaction. If the --dry-run flag is not set, additionally slack credentials are required. A message is sent to slack and the transaction is posted to a safe. If --dry-run is set, no message is sent nor is the transaction posted.
  • Without the --post-tx flag, another code path is triggered. It does contain the manual_propose function. The --dry-run flag now does not have any effect. The manual propose function does not require a signing key nor slack credentials. Instead, a csv is created. It can be used to manually propose a transaction via the safe ui.
  • The auto_propose function additionally adds an unwrap, which can fail if funds are missing. This sometimes requires to run the script twice, once without a flag (to check transfers) and once with flags --post-tx --dry-run to check if the creation of the transaction fails.

With this PR, there is an additional --send-to-slack flag.

  • If --post-tx is set, the additional flag has no impact.
  • If --post-tx is not set, a code path with the function manual_propose is taken. This function now also sometimes requires slack credentials. Which leads to some overlap between manual_propose and auto_propose.

A more appropriate set of flags might be:

  • --create-csv (which switches on the output of a csv)
  • --create-tx (which would behave like --post-tx --dry-run)
  • --post-to-safe (which requires --create-tx and would behave like --post-tx)
  • --post-to-slack (which would behave like --send-to-slack in case --post-tx is not set, and like --post-tx currently does otherwise)

The code would then have a function for each of these cases. Those functions would be called from some other function which implements appropriate if flag: do_stuff logic. Being able to switch on a csv output independently of the creation of a transaction will make testing changes easier.

@fhenneke fhenneke mentioned this issue Jan 29, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E:9.1 Streamline mainnet payouts process See https://github.com/cowprotocol/pm/issues/80 for details
Projects
None yet
Development

No branches or pull requests

1 participant