Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the most recent value in sats of an order calculation to the backend #224

Closed
KoalaSat opened this issue Sep 1, 2022 · 4 comments · Fixed by #235 or #236
Closed

Move the most recent value in sats of an order calculation to the backend #224

KoalaSat opened this issue Sep 1, 2022 · 4 comments · Fixed by #235 or #236

Comments

@KoalaSat
Copy link
Member

KoalaSat commented Sep 1, 2022

Is your feature request related to a problem? Please describe.
As discussed in #219 we want to move the most recent value in sats of an order calculation to the backend.

Describe the solution you'd like
I found these 2:

https://github.com/Reckless-Satoshi/robosats/blob/eff58dc91d93fe98a6738e0e3898ed2a476d2ebc/api/models.py#L382
https://github.com/Reckless-Satoshi/robosats/blob/eff58dc91d93fe98a6738e0e3898ed2a476d2ebc/api/models.py#L390

but the only logic for t0_satoshis is this:

https://github.com/Reckless-Satoshi/robosats/blob/eff58dc91d93fe98a6738e0e3898ed2a476d2ebc/api/views.py#L146

Looks like there was an original intention to keep the sats on creation and the sats right now, that's cool, but at some point it got lost and last_satoshis was used as t0_satoshis.

I like the original idea, I had a look and there are not too many places where last_satoshis is being used

https://github.com/Reckless-Satoshi/robosats/search?q=last_satoshis

Is it a good idea to try a renaming? so we use t0_satoshis everywhere and start calculating last_satoshis on every request?

Describe alternatives you've considered
We might just create a new parameter and leave the other params as they are.

Additional context
I would create 2 separate PR. First one that fix the naming. And a second one that actually adds the new calculation and expose it on the endpoint.

@Reckless-Satoshi
Copy link
Collaborator

Indeed, we keep track of t0_satoshis (simply noting them down). It might be truem that after that, t0_satoshis sees no use, altough, their intended use was to for validation that the order is not larger than the max order allowed (we might have used last_satoshis as it made no difference)

last_satoshis is only updated in the few requests that need them to be updated at the moment. That is: when the order is created and when the order is taken and the bond locked. It makes sense we might not want to update this value anymore after the order is taken. Therefore, for simplicity, I also think it might be best to create one more variable that we keep updated on every request, it can use the same function Logics.satoshis_now().

@KoalaSat
Copy link
Member Author

KoalaSat commented Sep 4, 2022

I have been revisiting this proposal, and I'm staring to consider I was wrong about the need of having satoshis_now, as you asked, there is no need to have the amount of satoshis now, since it's not affected by the actual price but by the amount at the asking price.

@KoalaSat KoalaSat closed this as completed Sep 4, 2022
@KoalaSat KoalaSat reopened this Sep 5, 2022
@KoalaSat
Copy link
Member Author

KoalaSat commented Sep 5, 2022

I also found we print the satoshis attribute, this time for this one it's always null, what's the use for this one @Reckless-Satoshi ?

@Reckless-Satoshi
Copy link
Collaborator

I also found we print the satoshis attribute, this time for this one it's always null, what's the use for this one @Reckless-Satoshi ?

The satoshis attribute is only used when the order pricing method "is explicit". That is, for orders where the amount of Sats are specified on the Maker Page instead of a premium over the market.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants