Emit portfolio event over WebSocket

[lukechilds]

How difficult would it be for you to emit a portfolio event over the WebSocket when the portfolio data changes?

I know we discussed previously (#562 (comment)) that for advanced Electrum support we'll need to implement it on our end. However that's a fairly big task and it's not going to be achievable in the timescales we're aiming for for the initial launch of our GUI. At the moment we're just polling mm with the portfolio method.

I was thinking as a temporary workaround it should fairly simple (I might be wrong here) for you to just emit a portfolio event whenever the portfolio values change. You are subscribed to the smart address balances on the mm end so when this changes you should receive an event from the electrum servers.

When you receive this event if you could just emit a portfolio event over the WebSocket we can listen for this and make sure our GUI balances are always in sync with mm.

Would that be something simple for you to implement?

[lukechilds]

To clarify, the data you send would just be the same data we receive when we manually issue a portfolio request.

So something like this:

{"queueid":0,"result":{"method":"portfolio","result":"success","kmd_equiv":10,"portfolio":[{"coin":"KMD","address":"<smartaddress>","amount":10,"price":1,"kmd_equiv":10,"perc":100,"goal":0,"goalperc":0,"relvolume":0,"force":0,"balanceA":10,"valuesumA":10,"aliceutil":100,"balanceB":10,"valuesumB":10,"balance":10,"bobutil":100}]}}

[jl777]

yes. keep in mind portfolio is a very time expensive call.
I was thinking it might be needed to have several different queues, which I prefer to avoid but if needed I can spawn parallel queues so you can group very long latency requests in their own independent queue

[lukechilds]

I thought portfolio was reading the balance values directly out of memory? It's normally one of the quicker calls in my test and responds in a few ms.

[jl777]

oh, in that case, nevermind. havent had to fix that part of the code in many months, so forget the details

[jl777]

i will add a "fast" flag and also whitelist orderbook and portfolio call to achieve a performant solution. you can experiment adding other api calls to the non-queued path by adding a "fast":1 flag and make sure to not have a "queueid" field for anything you want to immediately complete

[jl777]

pushed a new version to jl777 branch that supports dual command execution paths

the rule is if a command has "fast":1 or is orderbook or portfolio, it will run immediately, regardless of what other command is being processed
fasttest is a script that tests this:
curl --url "http://127.0.0.1:7783" --data "{"queueid":1,"userpass":"$userpass","method":"sleep","seconds":10}" &
curl --url "http://127.0.0.1:7783" --data "{"userpass":"$userpass","method":"orderbook","base":"REVS","rel":"KMD"}"
curl --url "http://127.0.0.1:7783" --data "{"userpass":"$userpass","method":"orderbook","base":"REVS","rel":"KMD"}"
curl --url "http://127.0.0.1:7783" --data "{"queueid":2,"userpass":"$userpass","method":"getcoin","coin":"REVS","rel":"KMD"}"
i also added a sleep api to create a call with definite duration
what the above does is it starts sleep async and it gets queued
while that is happening, I do 2 orderbooks, which immediately return.
after 10 seconds the sleep is done and the normal command queue completes and the getcoin is executed
this changes the serialized nature of calls, so it might destabilize things, but by being limited to orderbook and portfolio the risk is very small. new api can be added to the whitelisted fast calls, by testing it with a "fast":1

[lukechilds]

Thanks James, will give this a test 👍

[lukechilds]

This appears to be causing stability issues. If we have portfolio and orderbook polling and then we issue swapstatus to debug, it causes marketmaker to exit.

Are you able to either revert the change that automatically bypasses the queue, or alternatively make sure portfolio and orderbook are safe to happen in parallel with other commands?

[lukechilds]

Will try and get a backtrace on my Linux box and get that over to you.

[jl777]

If I knew why portfolio and orderbook are causing the crash, I might be able to make it safe. I can certainly revert the change, but then we get all the latency issues

[lukechilds]

Thread 7 "marketmaker" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffebfff700 (LWP 10830)]
0x00007ffff72af428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
54	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff72af428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff72b102a in __GI_abort () at abort.c:89
#2  0x00007ffff72f17ea in __libc_message (do_abort=do_abort@entry=2, 
    fmt=fmt@entry=0x7ffff740aed8 "*** Error in `%s': %s: 0x%s ***\n")
    at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007ffff72fa37a in malloc_printerr (ar_ptr=<optimized out>, 
    ptr=<optimized out>, 
    str=0x7ffff740afa0 "double free or corruption (fasttop)", action=3)
    at malloc.c:5006
#4  _int_free (av=<optimized out>, p=<optimized out>, have_lock=0)
    at malloc.c:3867
#5  0x00007ffff72fe53c in __GI___libc_free (mem=<optimized out>)
    at malloc.c:2968
#6  0x0000000000488053 in ?? ()
#7  0x0000000000412b64 in ?? ()
#8  0x000000000046d7dc in ?? ()
#9  0x000000000046d8cc in ?? ()
#10 0x000000000046d9ae in ?? ()
#11 0x00007ffff7bc16ba in start_thread (arg=0x7fffebfff700)
    at pthread_create.c:333
#12 0x00007ffff738141d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

[jl777]

this is some memory trashing... not any clues as to why it is crashing.
I reverted the orderbook/portfolio bypass in the jl777 branch. we need to get it stable again

[lukechilds]

Ok, thanks. Do you want me to try and create another backtrace?

[jl777]

if the latest fix is still crashing, that is bad and could mean something else completely is causing it. certainly a backtrace is always good if it is crashing

[lukechilds]

The above issue was caused by the portfolio/orderbook shortcut. Haven't noticed any instability since that has been reverted.