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

Priv call #250

Merged
merged 23 commits into from
Jan 7, 2020
Merged

Priv call #250

merged 23 commits into from
Jan 7, 2020

Conversation

pinges
Copy link
Contributor

@pinges pinges commented Dec 12, 2019

PR description

Fixed Issue(s)

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
@lucassaldanha lucassaldanha added the privacy private transactions label Dec 17, 2019
Copy link
Member

@lucassaldanha lucassaldanha left a comment

Choose a reason for hiding this comment

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

PR is on the right track! I've added some comments and suggestions.

@lucassaldanha
Copy link
Member

@pinges we need to remove the updated ethereum/referencetests/src/test/resources directory from the changes in this commit.

pinges and others added 17 commits December 27, 2019 14:51
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
}

@Test
public void privCallWithNonExistingPrivacyGroupMustNotSucceed() {
Copy link
Contributor

Choose a reason for hiding this comment

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

privCallWithoutExistingPrivacyGroupMustException
or
noExistingPrivacyGroupMustException
(as the class has the prefix PrivCall, also having as a method prefix seems redundant).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the test method names

@@ -27,7 +27,7 @@

public abstract class AbstractBlockParameterMethod implements JsonRpcMethod {

private final Supplier<BlockchainQueries> blockchainQueries;
protected final Supplier<BlockchainQueries> blockchainQueries;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why widen the scope (encapsulation) instead of using the getter (getBlockchainQueries)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a short while we need access to this. When we add the mainnet functionality this can be set back to private. I have added a TODO for making this private again.

pinges and others added 2 commits January 6, 2020 18:52
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
@pinges pinges marked this pull request as ready for review January 6, 2020 23:42
Copy link
Member

@lucassaldanha lucassaldanha left a comment

Choose a reason for hiding this comment

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

LGTM

@lucassaldanha lucassaldanha merged commit 6e67988 into hyperledger:master Jan 7, 2020
@pinges pinges deleted the priv_call branch January 7, 2020 00:36
edwardmack pushed a commit to ChainSafe/besu that referenced this pull request Feb 4, 2020
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
edwardmack pushed a commit to ChainSafe/besu that referenced this pull request Feb 4, 2020
Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>
Signed-off-by: edwardmack <ed@edwardmack.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
privacy private transactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants