-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add auto-refresh option #412
base: master
Are you sure you want to change the base?
Conversation
+1 for this PR. Would really like to have this. |
Fair enough. I love that it is backward-compatible and has to be turned on. utACK |
@CeruleanRat nice work! What's your thoughts on auto refresh of the transaction page when the transaction is unconfirmed? |
@pointbiz Thanks!
I think the right way to handle this would be to note the current block when the unconfirmed transaction is shown, and refresh the page (if auto-refresh is enabled and an unconfirmed transaction is being displayed) when a new block is detected - there's no need to refresh every n minutes in this case. I don't know if I will have the time to look at adding this myself just now; feel free to have a go if you'd like, otherwise I will see if I can find time to look at this over the next few weeks. I've had a very quick look at this and it seems a bit trickier than I'd have liked - it doesn't look like we know the current block height when we start to display the unconfirmed transaction. We could query it, but that's racy - imagine the transaction is confirmed just before we query the current block height, we would not then refresh the page until the next block. We could instead query the transaction's number of confirmations and refresh the page once it has some, but that would leave any "predicted in the next block" message up long after it's possibly untrue, so I suspect it's still better to trigger refresh based on block height, but the raciness of it feels ugly. I'll see if anything occurs to me. |
I've pushed an experimental implementation of the (slightly racy) block height approach here: https://github.com/CeruleanRat/btc-rpc-explorer/tree/auto-refresh-unconfirmed This has had minimal testing but seems to work for me; I'll keep running it on my own node and see how it goes, feel free to give it a test. |
Version 3.4 seems like a nice upgrade, thanks for your work on this! I have created a new branch https://github.com/CeruleanRat/btc-rpc-explorer/tree/auto-refresh-3.4 which implements the functionality of my auto-refresh branch for version 3.4. I am running this locally and it seems to work, but it hasn't had a lot of testing yet. (I haven't ported the slightly unsatisfactory "refresh of unconfirmed transactions" from the auto-refresh-unconfirmed branch to 3.4 yet.) |
tACK Also running this branch (auto-refresh-3.4) locally and it works flawlessly @ 68a5189. |
This change adds an auto-refresh option to the settings menu. It's disabled by default and unless you turn it on it shouldn't have any noticeable effects.
With auto-refresh enabled:
I managed to completely overlook the existence of pull request #77 before starting to work on this. I don't want to tread on Patrick's toes but since I did implement this I thought I'd submit a pull request anyway and see what you think. For what it's worth, it looks like I've taken a different approach; Patrick's code seems to be generating the block list on the client side, whereas my code is based on the idea of forcing refreshes when appropriate and leaving the server doing most of the work.
Let me know if you have any questions or comments, of course.
Cheers!
Steve