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

293: initial draft #295

Merged
merged 7 commits into from
Sep 3, 2018
Merged

293: initial draft #295

merged 7 commits into from
Sep 3, 2018

Conversation

JekaMas
Copy link
Contributor

@JekaMas JekaMas commented Aug 6, 2018

No description provided.

@b00ris b00ris changed the title [WIP ]293: initial draft 293: initial draft Aug 15, 2018
- QA
okrs:
- Core. [P0] LES and/or ULC are operational and used by at least 10% of all users.
- Research. Integrate and test ULC in a manner “graceful downgrade”: use infura by default and if it fails, start ULC node.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is Infura used by default? Seems complex, and if anything it should be the other way around. Am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

From my side, Infura is the lightest mode. It should be enough users, who want to use status as a chat. If you need to make transactions or communicate with smart contracts(or Infura is banned) you can switch to other options.

Copy link
Contributor

Choose a reason for hiding this comment

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

@b00ris I feel like graceful downgrade is a good UX feature, but it is a bit different and I doesn't belong to this swarm. If would be trivial to implement it if ULC and LES work reliably.

We need LES client enabled on mobile device to be really decentralized and to enable all web3 features for dApps.

### Requirements & Dependencies
Depends on research swarm #254
Copy link
Contributor

Choose a reason for hiding this comment

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

LES integration doesn't depend on this, it's bringing back previous work and fixing bugs around it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you are right. It blocks ULC integration. I've fixed it.

Depends on research swarm #254

### Security and Privacy Implications
* Free slots for LES servers
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you elaborate?


### Minimum Viable Product
* LES can work as the second Ethereum provider in status app.
* ULC can work as the third Ethereum provider in status app with static trusted nodes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think MVP is just LES, but perhaps the other iteration re ULC is concurrent? So we are working on both at the same time.

I don't understand why the goal date is in October? That's way too far away for initial LES (even if bad perf/a bit buggy) cc @mandrigin

Copy link
Contributor

Choose a reason for hiding this comment

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

If we are taking "just LES" as a first iteration (and in WIP), it should be done by Sep, 1st, I think.
Later, we can estimate how broken is it and what the next iterations are. Right now I know too little about the state of LES and ULC to make estimates for anything besides that.

Goal Date: 2018-08-27

Description:
* investigate les integration problems and fix them
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can be more precise here. I think it'd be useful to bring it back under a development flag as soon as possible, and figure out bugs as they are discovered. Doesn't seem realistic that all bugs will be fixed straight away, especially when they aren't clearly defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. I would put it as "return LES as a WIP option for testnets" as iteration 1 (I think that would be safer) and "make a list of issues of current LES implementation".
After this is done, we can really track progress and plan for that.
All the LES peers can be hardcoded for that, so we are putting discovery aside.


Description:
* run ULC compatible nodes in staus cluser
* integrate ULC to status-go as a patch
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why this is separate from ULC in go-ethereum master, isn't it essentially the same thing but two different outputs?

Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need a patch for status-go if it would be in geth already?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mandrigin @oskarth Removed.

Description:
* reserved for shifting

## Success Metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be useful to have some perf goals, for example see status-im/status-go#1025 re CHT and sync, and WiFi partial sync, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

@oskarth I'd avoid pre-optimizing anything. First, we need to make it work regardless of performance, then find performance issues and fix them. They shouldn't be in the plan, but I feel like it is too early to really thing about these ideas, while we don't really have proper problems set up.

Copy link
Contributor

Choose a reason for hiding this comment

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

@oskarth I agree with @mandrigin. I think it's a part of another swarm.

@b00ris
Copy link
Contributor

b00ris commented Aug 17, 2018

@oskarth Thank you for the comments. On it.

* run ULC compatible nodes in staus cluser
* integrate ULC to status-go as a patch

### Iteration 2018-09-10 - 2018-09-24
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't really plan behind the first iteration, because I personally have no idea of what needs to be done for LES to work properly :)

I would envision plan as

Iteration 1: add LES as a WIP option with a warning and make thorough testing of it.

Backlog:

  • ULC in go-ethereum;
  • ULC in status-go;
  • ULC on our cluster;
    ...

Copy link
Contributor

@mandrigin mandrigin left a comment

Choose a reason for hiding this comment

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

a few questions & suggestions

* TBD

## Success Metrics
* Status app can use LES and ULC provider instead of Infura.
Copy link
Contributor

Choose a reason for hiding this comment

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

Success metric is probably % of users using it

* Status app can use LES and ULC provider instead of Infura.

## Exit criteria
* Running on Mainnet
Copy link
Contributor

Choose a reason for hiding this comment

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

Being able to use app without major bugs like on Infura?


Idea: 293-ulc-integration
Title: ULC integration
Status: Draft
Copy link
Contributor

Choose a reason for hiding this comment

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

Still draft?

- mandrigin Clojure dev
- b00ris Go dev
- jeka Go dev
exit-criteria: no
Copy link
Contributor

Choose a reason for hiding this comment

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

yes right

success-metrics: no
clear-roles: no
future-iteration: no
roles-needed:
Copy link
Contributor

Choose a reason for hiding this comment

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

Still?

@mandrigin mandrigin self-assigned this Sep 1, 2018
@mandrigin
Copy link
Contributor

@oskarth I tweaked the idea, I think it is ready to be merged.

@oskarth oskarth merged commit 82338a3 into master Sep 3, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants