ethereum / ens-registrar-dapp

Registrar DApp for the Ethereum Name Service
MIT License
94 stars 56 forks source link

End filters when navigating away from a name. #153

Closed danfinlay closed 7 years ago

danfinlay commented 7 years ago

Currently it seems that the registrar Ðapp creates a number of filter subscriptions for each name viewed.

This is fairly expensive on bandwidth for remote providers, but is much worse because these filters seem to never be cancelled when navigating away, which should be part of the memory cleanup process, like deallocation.

At MetaMask we learned we weren't deallocating these as aggressively as we could, but the other half of the issue is Ðapps need to responsibly clean up.

evertonfraga commented 7 years ago

Hi @flyswatter, thanks for the heads up.

Currently I could only find two subscriptions throughout the code: HashRegisteredEvent.watch and AuctionStartedEvent.watch, which are set only once, and not for each name. Do you have any more detailed information about this?

If you felt that the Dapp was sluggish, maybe it'll be better after this PR: https://github.com/ethereum/ens-registrar-dapp/pull/151

evertonfraga commented 7 years ago

Oh, think I found some code smells here. I am new to this codebase, sorry for that. I'll look into it. Thanks!

evertonfraga commented 7 years ago

This PR should help, until a proper refactor. https://github.com/ethereum/ens-registrar-dapp/pull/156

danfinlay commented 7 years ago

Thanks @evertonfraga for putting some time into it!

I based this issue off observations of network requests being made through MetaMask as this dapp was being used, but your code links seem to suggest my theory was wrong, that was maybe just a MetaMask issue that I'm working to solve.

Closing this for now since the issue described seems to have been a mis-diagnosis of a symptom.