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

useFetchBlocks hooks refactor #529

Merged

Conversation

portdeveloper
Copy link
Contributor

@portdeveloper portdeveloper commented Sep 8, 2023

Description

Solves #528 . Also I have added wallet and test actions to the client with the websocket connection.
I removed the isLoading state too, because it already is loading so fast with wss, using that isLoading state actually makes the experience worse.
I plan to optimize and make the code for useFetchBlocks a little bit better before merging. Feel free to correct me and share your ideas!

Additional Information

Related Issues

_Closes #528 _

Note: If your changes are small and straightforward, you may skip the creation of an issue beforehand and remove this section. However, for medium-to-large changes, it is recommended to have an open issue for discussion and approval prior to submitting a pull request.

Your ENS/address: portdev.eth

@portdeveloper portdeveloper changed the title Usefetchblocks refactor useFetchBlocks hooks refactor Sep 10, 2023
@portdeveloper portdeveloper marked this pull request as ready for review September 17, 2023 17:46
@technophile-04
Copy link
Collaborator

Tysm @portdeveloper, this is looking great !! Added very minor comments

But everything looks and works great !!


Also noticed that yarn fork does not show any transactions on blockexplorer now, it was used to show things earlier if you wait a lot as mentioned in this #528 (comment) video

I think its because now we are using websockets and if you inspect chrome developers tools network tab I see bunch of "canceled" eth_blockNumber request when you yarn fork and go to blockexplorer page but not completely sure if its because of websockets

Nevertheless, I think these changes work great on Localchain and almost ready to merge 🙌

@technophile-04 technophile-04 linked an issue Sep 19, 2023 that may be closed by this pull request
1 task
@portdeveloper
Copy link
Contributor Author

Thanks @technophile-04 for your valuable feedback. I have fixed the things you have pointed out. After this is merged I will look into test actions so that the users can have a more fine grained control over their local environment. yarn fork is an issue too. I have tested it myself with a websocket connection to infura and the problem is that the useFetchBlocks, with its current logic, tries to get all blocks and all transactions in them before showing any.

Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @portdeveloper, I think it looks and works great now!

After this is merged I will look into test actions so that the users can have a more fine grained control over their local environment. yarn fork is an issue too. I have tested it myself with a websocket connection to infura and the problem is that the useFetchBlocks, with its current logic, tries to get all blocks and all transactions in them before showing any.

Yup, it would be really helpful if we could fix that so we only make requests for the latest 20 blocks and fetch their corresponding tx receipts & details that's it we shouldn't make request for other blocks, I will create an issue for this 🙌

But thanks for this PR always appreciate it !!

@technophile-04 technophile-04 merged commit ebfb874 into scaffold-eth:main Sep 21, 2023
1 check passed
@rin-st rin-st mentioned this pull request Sep 21, 2023
2 tasks
@github-actions github-actions bot mentioned this pull request Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Block explorer skips transactions
2 participants