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

Convert createTransactionBatch into an util #578

Open
dasanra opened this issue Nov 9, 2023 · 0 comments
Open

Convert createTransactionBatch into an util #578

dasanra opened this issue Nov 9, 2023 · 0 comments

Comments

@dasanra
Copy link
Collaborator

dasanra commented Nov 9, 2023

Context / issue

The need of creating a batch not only in the normal flow "createTransaction" but also in the counterfactual deployment created some duplicated logic that should be reviewed.

createTransactionBatch was intended to handle the batch encoding given a set of transactions which could be used as an util in the necessary methods. As it consumes several components form the Safe class that couldn't be referenced inside of an util at the moment the method was created first on the Safe class for speed and convenience.

Once the package was published, the method is exposed to developers which get it autosuggested when they type createTransaction adding confusion to which of those they should use as both accept an array of transactions.

Proposed solution

Refactor createTransactionBatch to be an internal util at transactions/utils
Change the function props to be an object containing the following:
contractManager, transactions, transactionOptions, callOnly = true

Passing the contractManager this.contractManager should be enough to remove the dependencies with all the properties of the Safe class.
Check the current batch implementation inside of createTransaction which uses this approach already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant