-
Notifications
You must be signed in to change notification settings - Fork 108
[sql-49] account + session: Add determintistic sorting of maps during the KVDB to SQL migration. #1132
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
Draft
ViktorTigerstrom
wants to merge
4
commits into
lightninglabs:master
Choose a base branch
from
ViktorTigerstrom:2025-08-session-migration-ceavat-order-fix-with-maps
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
[sql-49] account + session: Add determintistic sorting of maps during the KVDB to SQL migration. #1132
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
d3eb3cd
session: sort MacaroonRecipe.caveats in migration
ViktorTigerstrom 1af12f4
session: sort MacaroonRecipe.Permissions in migration
ViktorTigerstrom 3b51680
accounts: add deterministic map compare
ViktorTigerstrom 0e3eba8
session: add deterministic map compare
ViktorTigerstrom File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,11 +7,15 @@ import ( | |
"errors" | ||
"fmt" | ||
"reflect" | ||
"sort" | ||
"time" | ||
|
||
"github.com/btcsuite/btcd/btcec/v2" | ||
"github.com/davecgh/go-spew/spew" | ||
"github.com/lightninglabs/lightning-node-connect/mailbox" | ||
"github.com/lightninglabs/lightning-terminal/accounts" | ||
"github.com/lightninglabs/lightning-terminal/db/sqlc" | ||
"github.com/lightningnetwork/lnd/fn" | ||
"github.com/lightningnetwork/lnd/sqldb" | ||
"github.com/pmezard/go-difflib/difflib" | ||
"go.etcd.io/bbolt" | ||
|
@@ -24,6 +28,105 @@ var ( | |
"original session") | ||
) | ||
|
||
// featureConfigEntry is a variant of a single session feature config, which | ||
// can be inserted into an array to be compared deterministically. | ||
type featureConfigEntry struct { | ||
featureName string | ||
config []byte | ||
} | ||
|
||
// deterministicSession is a variant of the Session struct without any struct | ||
// methods, which represents the map in the Session as a list, so that it can be | ||
// deterministically sorted for comparison during the kvdb to SQL migration. | ||
type deterministicSession struct { | ||
ID ID | ||
Label string | ||
State State | ||
Type Type | ||
Expiry time.Time | ||
CreatedAt time.Time | ||
RevokedAt time.Time | ||
ServerAddr string | ||
DevServer bool | ||
MacaroonRootKey uint64 | ||
MacaroonRecipe *MacaroonRecipe | ||
PairingSecret [mailbox.NumPassphraseEntropyBytes]byte | ||
LocalPrivateKey *btcec.PrivateKey | ||
LocalPublicKey *btcec.PublicKey | ||
RemotePublicKey *btcec.PublicKey | ||
FeatureConfig []*featureConfigEntry | ||
WithPrivacyMapper bool | ||
PrivacyFlags PrivacyFlags | ||
|
||
// GroupID is the Session ID of the very first Session in the linked | ||
// group of sessions. If this is the very first session in the group | ||
// then this will be the same as ID. | ||
GroupID ID | ||
|
||
// AccountID is an optional account that the session has been linked to. | ||
AccountID fn.Option[accounts.AccountID] | ||
} | ||
|
||
// newDeterministicSession creates a deterministicSession from a Session struct. | ||
// This is used to compare the session in a deterministic way during the | ||
// migration from the KV database to the SQL database. | ||
func newDeterministicSession(sess *Session) *deterministicSession { | ||
var featuresConfig []*featureConfigEntry | ||
|
||
// If a session has a feature config set, we'll convert it to an array | ||
// so that we can sort it and compare it deterministically. | ||
if sess.FeatureConfig != nil { | ||
sessFC := *sess.FeatureConfig | ||
featuresConfig = make([]*featureConfigEntry, len(sessFC)) | ||
|
||
i := 0 | ||
for featureName, config := range sessFC { | ||
featuresConfig[i] = &featureConfigEntry{ | ||
featureName: featureName, | ||
config: config, | ||
} | ||
|
||
i++ | ||
} | ||
|
||
// Sort the feature config entries by their feature name, and | ||
// by their config bytes if the feature names are the same. | ||
sort.Slice(featuresConfig, func(i, j int) bool { | ||
iC := featuresConfig[i] | ||
jC := featuresConfig[j] | ||
|
||
if iC.featureName == jC.featureName { | ||
return bytes.Compare(iC.config, jC.config) < 0 | ||
} | ||
|
||
return iC.featureName < jC.featureName | ||
}) | ||
} | ||
|
||
return &deterministicSession{ | ||
ID: sess.ID, | ||
Label: sess.Label, | ||
State: sess.State, | ||
Type: sess.Type, | ||
Expiry: sess.Expiry, | ||
CreatedAt: sess.CreatedAt, | ||
RevokedAt: sess.RevokedAt, | ||
ServerAddr: sess.ServerAddr, | ||
DevServer: sess.DevServer, | ||
MacaroonRootKey: sess.MacaroonRootKey, | ||
PairingSecret: sess.PairingSecret, | ||
LocalPrivateKey: sess.LocalPrivateKey, | ||
LocalPublicKey: sess.LocalPublicKey, | ||
RemotePublicKey: sess.RemotePublicKey, | ||
GroupID: sess.GroupID, | ||
AccountID: sess.AccountID, | ||
PrivacyFlags: sess.PrivacyFlags, | ||
MacaroonRecipe: sess.MacaroonRecipe, | ||
WithPrivacyMapper: sess.WithPrivacyMapper, | ||
FeatureConfig: featuresConfig, | ||
} | ||
} | ||
|
||
// MigrateSessionStoreToSQL runs the migration of all sessions from the KV | ||
// database to the SQL database. The migration is done in a single transaction | ||
// to ensure that all sessions are migrated or none at all. | ||
|
@@ -148,13 +251,16 @@ func migrateSessionsToSQLAndValidate(ctx context.Context, | |
overrideSessionTimeZone(migratedSession) | ||
overrideMacaroonRecipe(kvSession, migratedSession) | ||
|
||
if !reflect.DeepEqual(kvSession, migratedSession) { | ||
dKvSession := newDeterministicSession(kvSession) | ||
dMigratedSession := newDeterministicSession(migratedSession) | ||
|
||
if !reflect.DeepEqual(dKvSession, dMigratedSession) { | ||
diff := difflib.UnifiedDiff{ | ||
A: difflib.SplitLines( | ||
spew.Sdump(kvSession), | ||
spew.Sdump(dKvSession), | ||
), | ||
B: difflib.SplitLines( | ||
spew.Sdump(migratedSession), | ||
spew.Sdump(dMigratedSession), | ||
), | ||
FromFile: "Expected", | ||
FromDate: "", | ||
|
@@ -165,7 +271,7 @@ func migrateSessionsToSQLAndValidate(ctx context.Context, | |
diffText, _ := difflib.GetUnifiedDiffString(diff) | ||
|
||
return fmt.Errorf("%w: %v.\n%v", ErrMigrationMismatch, | ||
kvSession.ID, diffText) | ||
dKvSession.ID, diffText) | ||
} | ||
} | ||
|
||
|
@@ -380,17 +486,69 @@ func overrideSessionTimeZone(session *Session) { | |
// as nil in the bbolt store. Therefore, we also override the permissions | ||
// or caveats to nil for the migrated session in that scenario, so that the | ||
// deep equals check does not fail in this scenario either. | ||
// | ||
// Additionally, we sort the caveats & permissions of both the kv and sql | ||
// sessions by their ID, so that they are always comparable in a deterministic | ||
// way with deep equals. | ||
func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) { | ||
if kvSession.MacaroonRecipe != nil { | ||
kvPerms := kvSession.MacaroonRecipe.Permissions | ||
kvCaveats := kvSession.MacaroonRecipe.Caveats | ||
|
||
// If the kvSession has a MacaroonRecipe with nil set for any | ||
// of the fields, we need to override the migratedSession | ||
// MacaroonRecipe to match that. | ||
if kvPerms == nil && kvCaveats == nil { | ||
migratedSession.MacaroonRecipe = &MacaroonRecipe{} | ||
} else if kvPerms == nil { | ||
migratedSession.MacaroonRecipe.Permissions = nil | ||
} else if kvCaveats == nil { | ||
migratedSession.MacaroonRecipe.Caveats = nil | ||
} | ||
|
||
sqlCaveats := migratedSession.MacaroonRecipe.Caveats | ||
sqlPerms := migratedSession.MacaroonRecipe.Permissions | ||
|
||
// If there have been caveats set for the MacaroonRecipe, | ||
// the order of the postgres db caveats will in very rare cases | ||
// differ from the kv store caveats. Therefore, we sort | ||
// both the kv and sql caveats by their ID, so that we can | ||
// compare them in a deterministic way. | ||
if kvCaveats != nil { | ||
sort.Slice(kvCaveats, func(i, j int) bool { | ||
return bytes.Compare( | ||
kvCaveats[i].Id, kvCaveats[j].Id, | ||
) < 0 | ||
}) | ||
|
||
sort.Slice(sqlCaveats, func(i, j int) bool { | ||
return bytes.Compare( | ||
sqlCaveats[i].Id, sqlCaveats[j].Id, | ||
) < 0 | ||
}) | ||
} | ||
|
||
// Similarly, we sort the macaroon permissions for both the kv | ||
// and sql sessions, so that we can compare them in a | ||
// deterministic way. | ||
if kvPerms != nil { | ||
sort.Slice(kvPerms, func(i, j int) bool { | ||
if kvPerms[i].Entity == kvPerms[j].Entity { | ||
return kvPerms[i].Action < | ||
kvPerms[j].Action | ||
} | ||
|
||
return kvPerms[i].Entity < kvPerms[j].Entity | ||
}) | ||
|
||
sort.Slice(sqlPerms, func(i, j int) bool { | ||
if sqlPerms[i].Entity == sqlPerms[j].Entity { | ||
return sqlPerms[i].Action < | ||
sqlPerms[j].Action | ||
} | ||
|
||
return sqlPerms[i].Entity < sqlPerms[j].Entity | ||
}) | ||
} | ||
Comment on lines
+534
to
+552
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loops for populating
invoices
andpayments
slices can be written more idiomatically in Go. Instead of manual index management, you could useappend
with slices initialized with a capacity. This would make the code more concise and less error-prone.For example: