-
Notifications
You must be signed in to change notification settings - Fork 308
Encapsulate parameters to proposeX methods #870
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| const msAccount = await this.getMultisigAccount(); | ||
| const newProposals = []; | ||
|
|
||
| const remote = targetCluster === undefined || targetCluster != this.cluster; |
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.
I think it should default to local if targetCluster is undefined
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.
LGTM, I tested on devnet and pythtest-crosschain. Please address comment
| return WORMHOLE_ADDRESS[this.cluster]; | ||
| } | ||
|
|
||
| // TODO: does this need a cluster argument? |
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.
Why would it need a cluster argument
This is a quick refactor to make this code a little easier to work with before I go about adding more functionality. The code right now instantiates a bunch of parameters in several different places (e.g., the SquadsMesh), which are needed to call
proposeInstructions. This change makes a class to hold all of these parameters so we can instantiate them all one time instead of repeating the code.I suggest looking at
propose.tsfirst. The other changes are pretty mechanical.