Notification multisig #752
Conversation
rg911
commented
Jan 25, 2021
rg911
commented
- Subscribe to multisig account from cosigner
- Fixed Add integrated service for multisig + cosigner ws listener #751
| ) { | ||
| this.url = url.replace(/\/$/, ''); | ||
| this.messageSubject = new Subject(); | ||
| this.multisigRepository = this.multisigRepository ? this.multisigRepository : new MultisigHttp(this.namespaceRepository.getUrl()); |
There was a problem hiding this comment.
Why asking for the url to the node repository? The URL is up there plus I would like to keep the repository URL agnostic.
There was a problem hiding this comment.
It's kind of hacky. I would make multisigRepository required like namespaceRepository and remove getUrl from the namespace repository. If somebody is already using the ListnerHttp, this person would know how to create a multisigRepository.
There was a problem hiding this comment.
is the way not to introduce a breaking change at this point of time
There was a problem hiding this comment.
getUrl is breaking the abstraction. Also why in the NamespaceRepository and not in the other ones (I'm not saying they should have it, it's just inconsistent).
If the devs coded the clients correctly, the construction of the Listener should be behind the scenes without breaking the code. If not, most people would know how to create the repositories used by the listener.
Those are my points, if you like to merge it's all good.
|
@rg911 , there is channel named 'modifyMultisigAccount'. But unused? are we missing listeners? |
rg911
force-pushed
the
notification_multisig
branch
from
0dc8ec8 to
a4696c0
Compare
* fix import path (#748) (#749) Co-authored-by: haryu703 <34744512+haryu703@users.noreply.github.com> * Added from / to height in receipt search (#750) * Notification multisig (#752) * - Subscribe multisig account from cosigner subscription - Fixed #751 * Lint fix * 0.23.1 release (#755) * 0.23.1 release node * updated versions Co-authored-by: haryu703 <34744512+haryu703@users.noreply.github.com>
* fix import path (#748) (#749) Co-authored-by: haryu703 <34744512+haryu703@users.noreply.github.com> * Added from / to height in receipt search (#750) * Notification multisig (#752) * - Subscribe multisig account from cosigner subscription - Fixed #751 * Lint fix * 0.23.1 release (#755) * 0.23.1 release node * updated versions * fixes finalizationEpoch (#758) * Fixed #754 (#757) * smal fix on mosaic restriction service * Fixed #760 (#761) - Removed NamespaceName api call - filter transaction by unrsolved address * Sub NamespaceId generation bug fix (#763) * Fixed #753 * Removed unused var * Add more unit tests Co-authored-by: fboucquez <fboucquez@gmail.com> * v0.23.2 release change log Co-authored-by: haryu703 <34744512+haryu703@users.noreply.github.com> Co-authored-by: Anthony Law <yc-law1015@hotmail.com> Co-authored-by: fboucquez <fboucquez@gmail.com>
