talaia-labs / rust-teos

The Eye of Satoshi - Lightning Watchtower
https://talaia.watch
MIT License
128 stars 62 forks source link

Add optional boundaries to getregistrationreceipt #199

Open ShubhamBhut opened 1 year ago

ShubhamBhut commented 1 year ago

fixes #97

@mariocynicys @sr-gi can you pls have a look at this ? I am actually pretty new to rust.

ShubhamBhut commented 1 year ago

Hey @sr-gi, Sincere apologies for the formatting mistakes. I believe this commit should solve everything; except I couldn't think of any new & better name for the updated methods.

sr-gi commented 1 year ago

There's something I just realized while manually testing this.

IIRC if two subscriptions overlapped, the user was only supposed to be storing the latest receipt. However, I just tested this and two receipts were returned where receipt_a ranged Ta-Tb, and receipt_b ranged Ta-Tc with c>b.

Does this ring a bell @mariocynicys?

No worries @ShubhamBhut, this is not due to your PR, but we may have forgotten to remove some data in an older PR.

mariocynicys commented 1 year ago

IIRC if two subscriptions overlapped, the user was only supposed to be storing the latest receipt. However, I just tested this and two receipts were returned where receipt_a ranged Ta-Tb, and receipt_b ranged Ta-Tc with c>b.

Revising this, we made the merger on the tower side and not the client side. The client side though gets it's subscription summary in memory updated to the merger of the two overlapping receipts, but all the receipts are stored in the DB nonetheless. We should delete those receipts since the last one already covers all of the past overlapping ones, so to get a cleaner output from getregistrationreceipt.

sr-gi commented 1 year ago

IIRC if two subscriptions overlapped, the user was only supposed to be storing the latest receipt. However, I just tested this and two receipts were returned where receipt_a ranged Ta-Tb, and receipt_b ranged Ta-Tc with c>b.

Revising this, we made the merger on the tower side and not the client side. The client side though gets it's subscription summary in memory updated to the merger of the two overlapping receipts, but all the receipts are stored in the DB nonetheless. We should delete those receipts since the last one already covers all of the past overlapping ones, so to get a cleaner output from getregistrationreceipt.

Referenced this here https://github.com/talaia-labs/rust-teos/issues/208

ShubhamBhut commented 1 year ago

I am having some issues with squashing, I will submit squashed PR after resolving them. I am submitting code for review in case my squashing issues remain longer.

sr-gi commented 1 year ago

@mariocynicys care to give this a look when you have some time?