graphsense / graphsense-REST

A REST service for accessing cryptocurrency data stored in Apache Cassandra.
MIT License
10 stars 8 forks source link

Hotfix/23.09 hotfix3 Fix address/entity Tx ordering #98

Closed myrho closed 10 months ago

soad003 commented 10 months ago

I have reviewed the changes. Looks good to me but its hard to test with complex cases with many transactions. I tried to add some more (edge) cases to the test suite.

Changes:

@myrho please have another look at the changes, esp. the merging logic. It might be worthwile to add some comments to explain and comment the code since its rather complex (esp. the loop in list_txs_ordered, and the parameters of the functions).

myrho commented 10 months ago

Still there are issues for eth and tokens. I'm on it.

myrho commented 10 months ago

Pretty nice cleanup! I just think we should consistently name the currency/network parameter either currency or network. Now we have both which might be confusing.

soad003 commented 10 months ago

@myrho I agree, i think over time we should switch to the name network, but its true that it might be confusing.

myrho commented 10 months ago

Would you like to align the naming for now?

soad003 commented 10 months ago

no lets rename the parameter network to currency and to the alignment outside of the context of this hotfix