Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 6 additions & 5 deletions firewall/request_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,19 +182,20 @@ func (r *RequestLogger) addNewAction(ctx context.Context, ri *RequestInfo,
withPayloadData bool) error {

// If no macaroon is provided, then an empty 4-byte array is used as the
// session ID. Otherwise, the macaroon is used to derive a session ID.
var sessionID [4]byte
// macaroon ID. Otherwise, the last 4 bytes of the macaroon's root key
// ID are used.
var macaroonID [4]byte
if ri.Macaroon != nil {
var err error
sessionID, err = session.IDFromMacaroon(ri.Macaroon)
macaroonID, err = session.IDFromMacaroon(ri.Macaroon)
if err != nil {
return fmt.Errorf("could not extract ID from macaroon")
}
}

actionReq := &firewalldb.AddActionReq{
SessionID: sessionID,
RPCMethod: ri.URI,
MacaroonIdentifier: macaroonID,
RPCMethod: ri.URI,
}

if withPayloadData {
Expand Down
16 changes: 12 additions & 4 deletions firewalldb/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/fn"
)

// ActionState represents the state of an action.
Expand Down Expand Up @@ -32,10 +33,17 @@ const (
// It contains all the information that is needed to create a new Action in the
// ActionStateInit State.
type AddActionReq struct {
// SessionID is the ID of the session that this action belongs to.
// Note that this is not serialized on persistence since the action is
// already stored under a bucket identified by the session ID.
SessionID session.ID
// MacaroonIdentifier is a 4 byte identifier created from the last 4
// bytes of the root key ID of the macaroon used to perform the action.
MacaroonIdentifier [4]byte
Copy link
Contributor

Choose a reason for hiding this comment

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

it's probably not worth it to have another type alias for [4]byte?

Copy link
Member Author

Choose a reason for hiding this comment

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

currently we dont use this in other places - so i think it's ok for now


// SessionID holds the optional session ID of the session that this
// action was performed with.
//
// NOTE: for our BoltDB impl, this is not persisted in any way, and we
// populate it by casting the macaroon ID to a session.ID and so is not
// guaranteed to be linked to an existing session.
SessionID fn.Option[session.ID]
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there's a test anywhere for an intercepted action, where we don't set this (i.e. it's not related to a session request).

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah i'll expand on the tests for SQL (where it actually will matter if it is set or not) once we've added the sql impl here 👍


// ActorName is the name of the entity who performed the Action.
ActorName string
Expand Down
8 changes: 5 additions & 3 deletions firewalldb/actions_kvdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/lightninglabs/lightning-terminal/session"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/tlv"
"go.etcd.io/bbolt"
)
Expand Down Expand Up @@ -80,7 +81,7 @@ func (db *BoltDB) AddAction(_ context.Context,
}

sessBucket, err := actionsBucket.CreateBucketIfNotExists(
action.SessionID[:],
action.MacaroonIdentifier[:],
)
if err != nil {
return err
Expand Down Expand Up @@ -109,7 +110,7 @@ func (db *BoltDB) AddAction(_ context.Context,
}

locator = kvdbActionLocator{
sessionID: action.SessionID,
sessionID: action.MacaroonIdentifier,
actionID: nextActionIndex,
}

Expand Down Expand Up @@ -549,7 +550,8 @@ func DeserializeAction(r io.Reader, sessionID session.ID) (*Action, error) {
return nil, err
}

action.SessionID = sessionID
action.MacaroonIdentifier = sessionID
action.SessionID = fn.Some(sessionID)
action.ActorName = string(actor)
action.FeatureName = string(featureName)
action.Trigger = string(trigger)
Expand Down
23 changes: 13 additions & 10 deletions firewalldb/actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/lightningnetwork/lnd/clock"
"github.com/lightningnetwork/lnd/fn"
"github.com/stretchr/testify/require"
)

Expand All @@ -18,7 +19,8 @@ var (
sessionID2 = intToSessionID(2)

action1Req = &AddActionReq{
SessionID: sessionID1,
SessionID: fn.Some(sessionID1),
MacaroonIdentifier: sessionID1,
ActorName: "Autopilot",
FeatureName: "auto-fees",
Trigger: "fee too low",
Expand All @@ -35,13 +37,14 @@ var (
}

action2Req = &AddActionReq{
SessionID: sessionID2,
ActorName: "Autopilot",
FeatureName: "rebalancer",
Trigger: "channels not balanced",
Intent: "balance",
RPCMethod: "SendToRoute",
RPCParamsJson: []byte("hops, amount"),
SessionID: fn.Some(sessionID2),
MacaroonIdentifier: sessionID2,
ActorName: "Autopilot",
FeatureName: "rebalancer",
Trigger: "channels not balanced",
Intent: "balance",
RPCMethod: "SendToRoute",
RPCParamsJson: []byte("hops, amount"),
}

action2 = &Action{
Expand Down Expand Up @@ -171,7 +174,7 @@ func TestListActions(t *testing.T) {
actionIds++

actionReq := &AddActionReq{
SessionID: sessionID,
MacaroonIdentifier: sessionID,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we set SessionID here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

want to keep the tests as close to how they are today as possible - then will expand the tests to cover more cases once the SQL impl is added 👍

ActorName: "Autopilot",
FeatureName: fmt.Sprintf("%d", actionIds),
Trigger: "fee too low",
Expand All @@ -194,7 +197,7 @@ func TestListActions(t *testing.T) {
require.Len(t, dbActions, len(al))
for i, a := range al {
require.EqualValues(
t, a.sessionID, dbActions[i].SessionID,
t, a.sessionID, dbActions[i].MacaroonIdentifier,
)
require.Equal(t, a.actionID, dbActions[i].FeatureName)
}
Expand Down
7 changes: 6 additions & 1 deletion session_rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,13 @@ func (s *sessionRpcServer) ListActions(ctx context.Context,
return nil, err
}

var sessionID session.ID
a.SessionID.WhenSome(func(id session.ID) {
sessionID = id
})

resp[i] = &litrpc.Action{
SessionId: a.SessionID[:],
SessionId: sessionID[:],
ActorName: a.ActorName,
FeatureName: a.FeatureName,
Trigger: a.Trigger,
Expand Down