-
Notifications
You must be signed in to change notification settings - Fork 32
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
fix: send exn to actual recipient only #273
base: main
Are you sure you want to change the base?
Changes from 5 commits
e62945c
f0762b6
8dd1fad
93e4586
000fd57
ff4c0ad
02f178a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,24 +78,19 @@ export class Exchanges { | |
payload: Dict<any>, | ||
embeds: Dict<any>, | ||
recipients: string[] | ||
): Promise<any> { | ||
for (const recipient of recipients) { | ||
): Promise<any[]> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should get proper types here instead of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this actually now returns an array of However, a better option could be to change this method to have one recipient and adjust from the call site accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, it was right and i incorrectly refactored and TS didnt pick it up due to any - will fix. But not so sure on changing the method to only have one recipient as this brings a lot of for loops at the call sites, which will make every wallet/app using multi-sig need to refactor everywhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and i broke tests again - weird, seems adding the end role doesn't like these changes |
||
return recipients.map(async (recipient) => { | ||
const [exn, sigs, atc] = await this.createExchangeMessage( | ||
sender, | ||
route, | ||
payload, | ||
embeds, | ||
recipient | ||
); | ||
return await this.sendFromEvents( | ||
name, | ||
topic, | ||
exn, | ||
sigs, | ||
atc, | ||
recipients | ||
); | ||
} | ||
return this.sendFromEvents(name, topic, exn, sigs, atc, [ | ||
recipient, | ||
]); | ||
}); | ||
} | ||
|
||
/** | ||
|
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.
Changes here for audit check on pipeline