Add support for keybase wallet listen

[nathanmsmith]

Introduces a new method on bot.wallet:

watchForNewTransactions(onTransaction: OnTransaction, onError?: OnError): Promise<void>;

Use and implementation is pretty similar to watchForNewMessages.

Under the hood, this method uses keybase chat api-listen --wallet. I debated a couple different approaches to incorporating it into the bot: one was to create a new method dedicated to transaction notifications, that filtered out chat and other messages, the other was to revamp watchForNewMessages to handle both chat and wallet notifications. This latter approach would have meant breaking the existing api and requiring all users to filter between chat/wallet notifications, and the objects were structured completely differently. My guess is that most users are only interested in one of the above, and for those interest in both, they plan on handling the notifications in different ways so the former approach made the most sense.

[malgorithms]

@nathunsmitty - will this provide info on whether the transaction was:

• attached to a chat message (e.g., 'hey +1xlm for ya' - and if so, the message info?)
• inside chat, but as a regular send (e.g., maybe there's an encrypted memo)
• or from outside chat entirely, like possibly from elsewhere on the stellar network.

it seems like we want to make sure it picks up everything and has good context. especially for the inline sends, as a lot of bots will be powered by inline chats @faxbot - send /keybase/private/chris,bot12/foo.pdf to 212-444-333 +10xlm

[nathanmsmith]

No to 1 for watchForNewTransactions. I believe it can distinguish between sends from a Keybase user vs. a Stellar account id, which would satisfy 2 and 3.

Funnily enough, we actually get data that can answer 1 through watchAllChannelsForNewMessages (i.e., keybase chat api-listen without the --wallet), as it has info about transactions associated with messages.

[malgorithms]

I think we should have the core side give us what we need before we merge/deploy this, so it's not missing transaction info, then. just so we get it right. can you make a ticket for them?

[malgorithms]

(in triage?)

[ddworken]

@malgorithms It looks like all of the relevant information is already exposed if one listens to the stream of both wallet events and chat events.

The chat events that initiate a transaction contain a transaction ID. All of the wallet events contain this transaction ID. So it is possible to correlate these two streams with each other in order to find the chat message associated with a transaction.

Since both wallet messages come from gregor it probably isn't a good idea to place any of this data into the gregor. This means that if this were to be implemented, it would have to be done by some sort of cache that correlates the chat messages (coming from gregor) with wallet messages (also coming from gregor). Eg this code which does "work":

But if there was a missed/dropped gregor message or they arrived out of order, it would break. Which to me seems like a really bad thing to build and provide to bots doing financial work. Instead, I'd envision a bot would want to subscribe to both wallet messages and chat messages and do this correlation themselves (with a timeout of some sort to handle out of order messages).

And also it is worth confirming that it is already possible to access the encrypted note and public memos associated with a transaction.

@nathunsmitty and I talked about this and our pitch is to instead have three methods for the bot library: watchForNewMessages, watchForNewPayment, and watchForNewEvent. This would allow bots that need the ability to correlate a chat message with a transaction to build it on top of watchForNewEvent. What do you think?

[malgorithms]

hmmm, I'd say for now let's not worry about this, even though I'm not comfortable with the proposal. Let's just leave it as you have it, where a user has to watch for transactions to get in-wallet or in-chat sends, but has to watch chat to get inline sends (and then check the transaction status).

I assume watchForNewEvent the same thing as the other 2 calls, just both of them in one stream? that doesn't sound so helpful, and I'd say it's just adding more confusion.

I know all the info is there - but I'm saying we shouldn't be implementing the same thing in every bot library we write (in each language), where we have to splice together 2 different streams of info and inspect everything to match them up. (or worse, expect our users to for a common use case.)

But if there was a missed/dropped gregor message or they arrived out of order, it would break. Which to me seems like a really bad thing to build and provide to bots doing financial work. Instead, I'd envision a bot would want to subscribe to both wallet messages and chat messages and do this correlation themselves (with a timeout of some sort to handle out of order messages).

^ this is exactly the kind of thing we wouldn't want everyone to have to implement themselves. If we're worried it's error prone when we write it, then no one else can write it, so we should be fixing it so it works reliably at the core level , and doesn't need to be re-implemented in each bot library (or worse, outside the bot library).

but I don't want to make perfection the enemy of good, so let's get this in as it is.