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

rules+session_rpc: use existing privacy mapper for obfuscating rules of linked sessions #637

Merged

Conversation

ellemouton
Copy link
Member

In this PR, we ensure that if we are linking a session to a previous one, that we extract any existing real-to-pseudo pairs from the privacy map DB instead of generating new ones.

@ellemouton ellemouton force-pushed the usePrevPrivMapForLinkedRules branch from 7f5c99a to 9bd69b6 Compare August 30, 2023 17:06
Copy link
Contributor

@ViktorTigerstrom ViktorTigerstrom left a comment

Choose a reason for hiding this comment

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

LGTM 🔥 🚀 ! Great find and fix to both of you :)! Leaving only a small comment regarding the unit test.

rules/channel_restrictions_test.go Outdated Show resolved Hide resolved
rules/peer_restrictions_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

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

Great changes 🍀! LGTM

@ellemouton ellemouton force-pushed the usePrevPrivMapForLinkedRules branch 2 times, most recently from 44a2ab9 to be65af1 Compare August 31, 2023 12:11
Copy link
Contributor

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

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

Cool, I like that new approach 👍🚀, will still test a bit.

rules/channel_restrictions.go Outdated Show resolved Hide resolved
rules/peer_restrictions.go Outdated Show resolved Hide resolved
rules/channel_restrictions_test.go Outdated Show resolved Hide resolved
@ellemouton ellemouton force-pushed the usePrevPrivMapForLinkedRules branch from be65af1 to 9d1cf4e Compare August 31, 2023 13:12
@ellemouton ellemouton requested a review from bitromortac August 31, 2023 13:12
Copy link
Contributor

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

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

LGTM, tACK 🎉 nice!

Copy link
Contributor

@ViktorTigerstrom ViktorTigerstrom left a comment

Choose a reason for hiding this comment

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

New fixes looks good 🚀 !
Leaving some suggestions for the PrivacyMapPairs.Add that's non-blocking, and one small issue that I think should be fixed.
Amazed at the speed you're implementing these fixes @ellemouton, it's incredibly impressive 😃!!

firewalldb/privacy_mapper.go Outdated Show resolved Hide resolved
firewalldb/privacy_mapper.go Outdated Show resolved Hide resolved
firewalldb/privacy_mapper.go Show resolved Hide resolved
firewalldb/privacy_mapper_test.go Show resolved Hide resolved
This commit adds a new FetchAllPairs to the PrivacyMapTx interface. This
method returns a new PrivacyMapPairs struct which is an in-memory
privacy map DB. The PrivacyMapPairs struct implements a new
PrivacyMapReader interface which can be used to pass around read only
access to the PrivacyMapPairs struct.
This commit expands the RealToPseudo methods to take in a privacy map db
reader. This allows the methods to check if the privacy map db already
contains an entry for a "real" string before generating a new one.

For now, only an empty PrivacyMapReader is ever provided to the
RealToPseudo call. This will be changed in the following commit.
In this commit, we keep track of all known privacy map pairs for a
session along with any new pairs to be persisted.
@ellemouton ellemouton force-pushed the usePrevPrivMapForLinkedRules branch from 9d1cf4e to 7bb4d3b Compare August 31, 2023 14:57
Copy link
Contributor

@ViktorTigerstrom ViktorTigerstrom left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! LGTM 🚀:fire::rocket:!!

@ellemouton ellemouton merged commit 71a57a5 into lightninglabs:master Sep 1, 2023
@ellemouton ellemouton deleted the usePrevPrivMapForLinkedRules branch September 1, 2023 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants