Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

Light client #1547

Closed
wants to merge 9 commits into from
Closed

Light client #1547

wants to merge 9 commits into from

Conversation

alexvandesande
Copy link

@alexvandesande alexvandesande commented Dec 22, 2016

Second PR, focusing on light client issues only.

Review but do not merge before more group discussion.

@alexvandesande
Copy link
Author

alexvandesande commented Dec 22, 2016

goes better after #1545

@@ -130,7 +139,7 @@ class EthereumNode extends EventEmitter {
log.info(`Network: ${this.defaultNetwork}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add: log.info(Mode: ${this.defaultMode});


return this.stop()
.then(() => {
Windows.loading.show();
})
.then(() => {
return this._start(newType || this.type, newNetwork || this.network);
return this._start(newType || this.type, newNetwork || this.network, newMode || this.nodeMode);
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats this this.nodeMode ? i never see this variable created,shouldnt it be this.mode or something?

@alexvandesande
Copy link
Author

@frozeman addressed both issues

@@ -162,7 +163,7 @@ class EthereumNode extends EventEmitter {
Windows.loading.show();
})
.then(() => {
return this._start(newType || this.type, newNetwork || this.network, newMode || this.nodeMode);
return this._start(newType || this.type(), newNetwork || this.network(), newMode || this.mode());
Copy link
Contributor

Choose a reason for hiding this comment

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

thats not functions, this are get variables.
So just this.mode

Copy link
Author

Choose a reason for hiding this comment

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

I've tested and this.mode() was returning a value and this.mode wasn't. Are you sure this isn't being made into a function by https://github.com/ethereum/mist/pull/1547/files#diff-409860c680be3333b2089d577f193cf5R65 ?

Will test more this part

@evertonfraga
Copy link
Member

  • When the light client is far behind and I'm on the app screen (not splash), mist hits high CPU usage until the node catches up. That may cause unresponsiveness on slower machines (mine is a quad-core 2.2 GHz)
  • Mist slowness was on the main thread, making the actual interface sluggish
  • When I'm on light client mode and disconnect from internet and connect again, it didn't seem to automatically reconnect – or there weren't many peers available on my tests.

@alexvandesande
Copy link
Author

superseded by #3052

@lock
Copy link

lock bot commented Mar 29, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Mar 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants