Token amount history #498
Token amount history #498
Conversation
We reset here the inputs that have tokens, so they are re-updated and token's input will be inserted.
We redo the exact same way as for the ALPH amount history, but instead of sum up the input/output we sum up the token_input/token_output.
|
What is the query complexity of this endpoint? It looks like very expensive to support |
|
It's the same as the complexity as the one we already have to show the amount history for ALPH, it's used in the wallet to show the graph evolution of price. for For each of those time intervals we sum the input and output amount, separately so no join. so it's a linear pass in both table. There are indexes on timestamp so don't need to pass the all tables. So 2 tables, max 365 times a linear query. |
| run( | ||
| for { | ||
| in <- sumAddressTokenInputs(address, token, from, to) | ||
| out <- sumAddressTokenOutputs(address, token, from, to) | ||
| } yield (in, out, to) |
There was a problem hiding this comment.
Just a question: The two tables being queried here are token_output and token_inputs, these tables are inserted by BlockFlowSyncService within this function, non-transactionally and also concurrently. Could there be a situation where for an address, the above two sum queries return more for the sum of token-output than token-inputs or vice versa? Or in general, could it return out of sync data for input and output for an address?
There was a problem hiding this comment.
indeed, it could have a small time interval where it could happen. I need to check if we could have insertBlocks as a DBAction rather than a Future, otherwise it will be complicated to have a transactionally query
There was a problem hiding this comment.
mmm it's complicated to move to full DBAction, during the insert, if we miss the parent we need to recover that and fetch the missing parent from the node, so it's a Future.
There was a problem hiding this comment.
So this is a bug right? The possibility that the explorer could display n outputs for an address with n - 1 input? Should this be addressed?
| (intervalType.duration - Duration.ofMillisUnsafe(1)).get | ||
| ) | ||
| FlowableUtil.getAmountHistory(from, to, intervalType, paralellism) { case (fromTs, toTs) => | ||
| getInOutAmount(address, fromTs, toTs) |
There was a problem hiding this comment.
Another question: Since this query getInOutAmount is being invoked multiple times for a single address with different timestamps ranges, would it simpler and more efficient to fetch this data in a single query? Since address and timestamps are both indexed.
For example (untested):
SELECT SUM(output_ref_amount)
FROM inputs
WHERE ...
AND (block_timestamp BETWEEN $fromTime1 AND $toTime1)
OR (block_timestamp BETWEEN $fromTime2 AND $toTime2)
...Totally understandable if reactive stream is preferred instead, for back-pressure and other fancy stuff that comes with it.
There was a problem hiding this comment.
very good point, as this query is limited to max 365 days, I could give a try and check performance. Backpresure and streaming is more important for the txs exporting endpoint.
There was a problem hiding this comment.
I updated this in #499 I'll change that PR once merged
|
on hold until #499 is merged |
|
I'm closing as we discussed with the team and we will rather download the transactions history and get the token amount history from there |
Resolves #497
High priority as it blocks the next release of the wallet
It might looks like a huge PR, but it's mostly some functions extracted and some other copy/pasted and adapted from our current
amount-historyfor ALPH endpoint.The only "real" new part is the new
token_inputstable, we need it in order to compute the diff of token amount.For example the
sumAddressToken*are taken from here