Bug fix and improvement on marina events

[bordalix]

Closes #377

Bug fixes:

• fix bug where only the main account was emitting events on NEW_TX, NEW_UTXO and SPENT_UTXO

Improvements:

• marina events (see list above) now returns the accountID in the payload

Breaking changes (marina events):

• marina.on('NEW_TX', (tx) => {}) is now marina.on('NEW_TX', ({ tx, accountID }) => {})
• marina.on('NEW_UTXO', (utxo) => {}) is now marina.on('NEW_UTXO', ({ utxo, accountID }) => {})
• marina.on('SPENT_UTXO', (utxo) => {}) is now marina.on('SPENT_UTXO', ({ utxo, accountID }) => {})

Worth mentioning:

• adds accountID to console.debug when reporting utxos fetched from explorer

@tiero @louisinger please review.

[tiero]

If we pass an additional parameter, instead of changing it to be an object, we should be able to make it not breaking change?

marina.on('NEW_TX', (tx, accountID?) => {})

[bordalix]

If we pass an additional parameter, instead of changing it to be an object, we should be able to make it not breaking change?

marina.on('NEW_TX', (tx, accountID?) => {})

@louisinger help me out here please, but I think this will implicate a profound change on Marina. Today, the payload for the events is passed between components as a single object (which gets stringified). @tiero what you ask for would imply to have multiple objects being passed through.

[bordalix]

@tiero to avoid more breaking changes, all payloads should be on the form { network } instead of network. This way we can add more attributes without breaking previous code.

[tiero]

Agreed. I don't see any other way.

I woudl eventually make a typed interface:

interface MarinaEvent {
  accountID: string,
  data: any
}

[louisinger]

If we pass an additional parameter, instead of changing it to be an object, we should be able to make it not breaking change?

marina.on('NEW_TX', (tx, accountID?) => {})

it means update the MarinaProvider interface (the handler func type of on method)

But anyway not sure how marina background broker can handle this. if user has passed a 1-param func previously, func(a, undefined) statement will throw an error anyway. I personally prefer passing an object like joao proposes.

PS: the only way to avoid breaking changes is to return the accountID inside the current payload, so in NEW_UTXO we return { ...utxo, accountID } instead of utxo. The drawback is utxo type don't have the accountID member so it's a kind of "hidden" member from a TS point of view.

PPS: note that due to payload typing which depends on the event type, the "breaking change" is a silent one -> it means that linter and TS checks won't see the error (even if a good code should check the type returned by marina)

[bordalix]

Final merge was:

interface MarinaEvent {
    accountID?: string
    data: any
}

With data type depending on event type:

marina.on('NEW_TX', ({ accountID, data }) => {}) // data: Transaction
marina.on('NEW_UTXO', ({ accountID, data }) => {}) // data: UnblindedOutput
marina.on('SPENT_UTXO', ({ accountID, data }) => {}) // data: UnblindedOutput
marina.on('ENABLED', ({ data }) => {}) // data: { hostname: string; network: NetworkString }
marina.on('DISABLED', ({ data }) => {}) // data: { hostname: string; network: NetworkString }
marina.on('NETWORK', ({ data }) => {}) // data: NetworkString

Updated on https://docs.vulpem.com/marina/api/#marina-events