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

Dapps-specific accounts #3627

Merged
merged 8 commits into from
Dec 4, 2016
Merged

Dapps-specific accounts #3627

merged 8 commits into from
Dec 4, 2016

Conversation

tomusdrw
Copy link
Collaborator

Initial take on #3382

  1. Meta information about the dapp is derived from Referer header (it can be spoofed though, we need to figure out something better)
  2. Additional DappId parameter is added to eth_accounts via Middleware (I have a much better approach in mind, but need to significantly rewrite jsonrpc-core for that, will follow in next PRs)
  3. Using jsonrpc packages from ethcore/jsonrpc with updated hyper and rotor/mio/slab, should solve Parity 1.4.4 panics with "Tried to insert into filled index" Error #3545 too.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Nov 26, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 85.901% when pulling 3b595a0 on rpc-middleware into a7037f8 on master.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f5a8996 on rpc-middleware into ** on master**.

@gavofyork
Copy link
Contributor

can we get an issue for point 1 and mark F1-security and set for 1.6? i don't want to forget about this.

@@ -266,7 +266,7 @@ impl Server {
#[cfg(test)]
/// Returns address that this server is bound to.
pub fn addr(&self) -> &SocketAddr {
self.server.as_ref().expect("server is always Some at the start; it's consumed only when object is dropped; qed").addr()
&self.server.as_ref().expect("server is always Some at the start; it's consumed only when object is dropped; qed").addrs()[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

panicker: prove or remove!

what guarantee is that that there is at least one address in addrs? in any case better to use .front() to make this explicit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a test-only code, but proof added.

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Nov 29, 2016
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Nov 29, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling df3c07b on rpc-middleware into ** on master**.

@gavofyork gavofyork added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 1, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 85.931% when pulling 1dd1337 on rpc-middleware into 21a76c2 on master.

@gavofyork gavofyork merged commit 0c7b7fc into master Dec 4, 2016
@gavofyork gavofyork deleted the rpc-middleware branch December 4, 2016 16:46
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Dec 4, 2016
@tomusdrw tomusdrw added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Dec 10, 2016
@gavofyork gavofyork removed the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Dec 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants