-
Notifications
You must be signed in to change notification settings - Fork 3
feat: GET /historical_fee #6
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
Conversation
app/src/main/kotlin/xyz/block/augurref/api/FeeEstimateEndpoint.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/xyz/block/augurref/api/FeeEstimateEndpoint.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/xyz/block/augurref/api/FeeEstimateEndpoint.kt
Outdated
Show resolved
Hide resolved
55ae962 to
c176e20
Compare
|
Btw, I'm using this to create a chart like this, and my goal is to then compare the estimates to what happened on-chain to be able to chart out weekly the accuracy with live data. I also just think it would be useful to be able to reference historical estimations One final thought is that Augur currently does not support sub 1 sat/vbyte transactions and Bitcoin-Core should be dropping its mintxrelay amount bitcoin/bitcoin#33106, so it might be useful to also do the same for Augur |
|
nice work @kevkevinpal! this is definitely a change we'll want to merge in. some feedback:
|
|
Just a heads up that I'm going to take this out of draft mode so that I can follow the conversation between you and @rloomba. We have some GitHub slack notifications in place that I'm pretty sure don't notify us when things are in draft mode, and we'd like to be able to follow new activity 😄 |
1). Yea I can do that, was not sure if a new endpoint or building on the current one was preferred. Either is fine for me
2). I think when implementing the date param was just simpler and fit how Maybe having multiple two endpoints might make sense I can work on the singular version for this PR and open another PR for a plural/multiple historical fees version |
cb70f55 to
fa51beb
Compare
|
So I went ahead and created let me know what you think and if there are any modifications you'd like me to do and I can work on it! |
app/src/main/kotlin/xyz/block/augurref/service/MempoolCollector.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/xyz/block/augurref/service/MempoolCollector.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/xyz/block/augurref/service/MempoolCollector.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/xyz/block/augurref/service/MempoolCollector.kt
Outdated
Show resolved
Hide resolved
977c25c to
3c3757d
Compare
|
nice work on this @kevkevinpal! will review this in a day or two |
| */ | ||
| fun getFeeEstimateForDate(dateTimeUTC: LocalDateTime): FeeEstimate? { | ||
| val dateTime = dateTimeUTC | ||
| .atZone(ZoneId.of("UTC")) // Interpret as UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the logic to handle timezones is overly complex, what do you think about just using UTC timestamps instead?
// Single historical estimate
GET /historical_fee?timestamp=1723128639
// Date range with interval in seconds
GET /historical_fees?start_timestamp=1723128639&end_timestamp=1723214639&interval=3600
|
fcc12bc looks pretty good to me. What do you think about breaking that into it's own PR so we can get that merged quicker. I think it just needs:
for |
3c3757d to
fae5612
Compare
fae5612 to
eac411d
Compare
app/src/main/kotlin/xyz/block/augurref/api/HistoricalFeeEndpoint.kt
Outdated
Show resolved
Hide resolved
eac411d to
e765828
Compare
|
This looks great, only feedback is, let's change all variable names from I'm thinking: date -> timestamp |
|
and let's get some example curl commands for this endpoint added to the README |
e765828 to
a65f8ff
Compare
a65f8ff to
ef26ad2
Compare
rloomba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically ready for merge! one final task:
- example curl request/response in the README.MD
Signed-off-by: kevkevinpal <oapallikunnel@gmail.com>
Signed-off-by: kevkevinpal <oapallikunnel@gmail.com>
Signed-off-by: kevkevinpal <oapallikunnel@gmail.com>
59782d5 to
473787b
Compare
oops ya just added an example request/response here |
rloomba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job!!



Description
related and closes #2
I wanted a way to retrieve historical estimates.
This PR introduces a new endpoint
GET /historical_feethat takes thedatequery parameter to query for historical fee estimates.Details
Add
date=<utc timestamp>query paramwill get you the same response as
GET /feesbut with an estimation from the provided dateExample request
http://100.97.107.124:8080/historical_fee?date=1755353169Example response
TODO
Note this is still a draft, I still need to do a few things, but wanted to put it out there for any draft staged review
Follow-up:
Create another endpoint for multiple historical fees (rebase after this merges) #11
GET /historical_fees?start_date=<utc timestamp>&end_date=<utc timestamp>&interval=<1h,20m,30s>