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

Private Log Filters track enclavePublicKey and are removed when user removed from group #1298

Merged

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Aug 12, 2020

PR description

When multi-tenancy and removing user from onchain privacy group, send an event which is processed by the FilterManager, which triggers removal of any private log filters created by that user in that privacy group.

Fixed Issue(s)

See #883

Changelog

Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
@macfarla macfarla added the TeamRevenant GH issues worked on by Revenant Team label Aug 12, 2020
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
…cy AND onchain groups enabled

Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
@macfarla macfarla marked this pull request as ready for review August 21, 2020 00:25

final PrivateTransactionEvent removalEvent =
new PrivateTransactionEvent(
privateTransaction.getPrivacyGroupId().get().toBase64String(), removedParticipant);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a chance for an NPE here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from within the onchain precompile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking at the code, the calling method returns earlier if privacy group id is empty. I can refactor to make that clearer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unchecked privateTransaction.getPrivacyGroupId().get() ?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right - I missed that in the diff.

Any chance this function could be called from a different path?

Copy link
Contributor

@mark-terry mark-terry left a comment

Choose a reason for hiding this comment

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

Looks pretty good - one comment.

Copy link
Contributor

@pinges pinges left a comment

Choose a reason for hiding this comment

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

LGTM

@macfarla macfarla requested a review from lucassaldanha August 21, 2020 05:03
@macfarla macfarla dismissed lucassaldanha’s stale review August 21, 2020 05:07

Changes have been made, and @pinges has approved

@macfarla macfarla merged commit b66d2e6 into hyperledger:master Aug 21, 2020
@macfarla macfarla deleted the multi-tenancy-onchain-remove-filter branch August 21, 2020 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TeamRevenant GH issues worked on by Revenant Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants