Skip to content

[sql-38] multi: couple sessions and accounts to actions #1071

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

Merged
merged 4 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 8 additions & 27 deletions accounts/interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (s *InterceptorService) Intercept(ctx context.Context,
return mid.RPCErrString(req, "error parsing macaroon: %v", err)
}

acctID, err := accountFromMacaroon(mac)
acctID, err := IDFromCaveats(mac.Caveats())
if err != nil {
return mid.RPCErrString(
req, "error parsing account from macaroon: %v", err,
Expand All @@ -91,15 +91,17 @@ func (s *InterceptorService) Intercept(ctx context.Context,
// No account lock in the macaroon, something's weird. The interceptor
// wouldn't have been triggered if there was no caveat, so we do expect
// a macaroon here.
if acctID == nil {
return mid.RPCErrString(req, "expected account ID in "+
"macaroon caveat")
accountID, err := acctID.UnwrapOrErr(
fmt.Errorf("expected account ID in macaroon caveat"),
)
if err != nil {
return mid.RPCErr(req, err)
}

acct, err := s.Account(ctx, *acctID)
acct, err := s.Account(ctx, accountID)
if err != nil {
return mid.RPCErrString(
req, "error getting account %x: %v", acctID[:], err,
req, "error getting account %x: %v", accountID[:], err,
)
}

Expand Down Expand Up @@ -208,27 +210,6 @@ func parseRPCMessage(msg *lnrpc.RPCMessage) (proto.Message, error) {
return parsedMsg, nil
}

// accountFromMacaroon attempts to extract an account ID from the custom account
// caveat in the macaroon.
func accountFromMacaroon(mac *macaroon.Macaroon) (*AccountID, error) {
if mac == nil {
return nil, nil
}

// Extract the account caveat from the macaroon.
accountID, err := IDFromCaveats(mac.Caveats())
if err != nil {
return nil, err
}

var id *AccountID
accountID.WhenSome(func(aID AccountID) {
id = &aID
})

return id, nil
}

// CaveatFromID creates a custom caveat that can be used to bind a macaroon to
// a certain account.
func CaveatFromID(id AccountID) macaroon.Caveat {
Expand Down
3 changes: 2 additions & 1 deletion config_dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
}

firewallBoltDB, err := firewalldb.NewBoltDB(
networkDir, firewalldb.DBFilename, stores.sessions, clock,
networkDir, firewalldb.DBFilename, stores.sessions,
stores.accounts, clock,
)
if err != nil {
return stores, fmt.Errorf("error creating firewall BoltDB: %v",
Expand Down
3 changes: 2 additions & 1 deletion config_prod.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
stores.closeFns["sessions"] = sessStore.Close

firewallDB, err := firewalldb.NewBoltDB(
networkDir, firewalldb.DBFilename, sessStore, clock,
networkDir, firewalldb.DBFilename, stores.sessions,
stores.accounts, clock,
)
if err != nil {
return stores, fmt.Errorf("error creating firewall DB: %v", err)
Expand Down
8 changes: 8 additions & 0 deletions firewall/request_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"strings"

"github.com/lightninglabs/lightning-terminal/accounts"
"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/lnrpc"
Expand All @@ -29,6 +30,7 @@ const (
// request.
type RequestInfo struct {
SessionID fn.Option[session.ID]
AccountID fn.Option[accounts.AccountID]
MsgID uint64
RequestID uint64
MWRequestType string
Expand Down Expand Up @@ -140,6 +142,12 @@ func NewInfoFromRequest(req *lnrpc.RPCMiddlewareRequest) (*RequestInfo, error) {
}
}

ri.AccountID, err = accounts.IDFromCaveats(ri.Macaroon.Caveats())
if err != nil {
return nil, fmt.Errorf("error extracting account ID "+
"from macaroon: %v", err)
}

return ri, nil
}

Expand Down
1 change: 1 addition & 0 deletions firewall/request_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ func (r *RequestLogger) addNewAction(ctx context.Context, ri *RequestInfo,

actionReq := &firewalldb.AddActionReq{
SessionID: ri.SessionID,
AccountID: ri.AccountID,
MacaroonIdentifier: macaroonID,
RPCMethod: ri.URI,
}
Expand Down
8 changes: 8 additions & 0 deletions firewalldb/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"time"

"github.com/lightninglabs/lightning-terminal/accounts"
"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/fn"
)
Expand Down Expand Up @@ -45,6 +46,13 @@ type AddActionReq struct {
// guaranteed to be linked to an existing session.
SessionID fn.Option[session.ID]

// AccountID holds the optional account ID of the account that this
// action was performed on.
//
// NOTE: for our BoltDB impl, this is not persisted in any way, and we
// do not populate it on reading from disk.
AccountID fn.Option[accounts.AccountID]

// ActorName is the name of the entity who performed the Action.
ActorName string

Expand Down
28 changes: 26 additions & 2 deletions firewalldb/actions_kvdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"time"

"github.com/lightninglabs/lightning-terminal/accounts"
"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/tlv"
Expand Down Expand Up @@ -54,9 +55,32 @@ var (
)

// AddAction serialises and adds an Action to the DB under the given sessionID.
func (db *BoltDB) AddAction(_ context.Context,
func (db *BoltDB) AddAction(ctx context.Context,
req *AddActionReq) (ActionLocator, error) {

// If the new action links to a session, the session must exist.
// For the bbolt impl of the store, this is our best effort attempt
// at ensuring each action links to a session. If the session is
// deleted later on, however, then the action will still exist.
var err error
req.SessionID.WhenSome(func(id session.ID) {
_, err = db.sessionIDIndex.GetSession(ctx, id)
})
if err != nil {
return nil, err
}
Comment on lines +69 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could potentially wrap the error on session.ErrSessionNotFound errors to make it clear that was a linked session for an action that wasn't found.

Copy link
Member Author

Choose a reason for hiding this comment

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

GetSession already handles that - no need to wrap again i'd say! This is proved by the unit tests asserting the error


// If the new action links to an account, the account must exist.
// For the bbolt impl of the store, this is our best effort attempt
// at ensuring each action links to an account. If the account is
// deleted later on, however, then the action will still exist.
req.AccountID.WhenSome(func(id accounts.AccountID) {
_, err = db.accountsDB.Account(ctx, id)
})
if err != nil {
return nil, err
}
Comment on lines +80 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
Same here but for accounts.ErrAccNotFound.

Copy link
Member Author

Choose a reason for hiding this comment

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

already handled - see unit tests that assert this


action := &Action{
AddActionReq: *req,
AttemptedAt: db.clock.Now().UTC(),
Expand All @@ -69,7 +93,7 @@ func (db *BoltDB) AddAction(_ context.Context,
}

var locator kvdbActionLocator
err := db.DB.Update(func(tx *bbolt.Tx) error {
err = db.DB.Update(func(tx *bbolt.Tx) error {
mainActionsBucket, err := getBucket(tx, actionsBucketKey)
if err != nil {
return err
Expand Down
Loading
Loading