Skip to content

multi: be consistent with UTC conversion of timestamp #976

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

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 1 addition & 1 deletion accounts/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (a *OffChainBalanceAccount) HasExpired() bool {
return false
}

return a.ExpirationDate.Before(time.Now())
return a.ExpirationDate.Before(time.Now().UTC())
}

// CurrentBalanceSats returns the current account balance in satoshis.
Expand Down
2 changes: 1 addition & 1 deletion cmd/litcli/autopilot.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func listFeatures(cli *cli.Context) error {

func initAutopilotSession(cli *cli.Context) error {
sessionLength := time.Second * time.Duration(cli.Uint64("expiry"))
sessionExpiry := time.Now().Add(sessionLength).Unix()
sessionExpiry := time.Now().UTC().Add(sessionLength).Unix()

ctx := getContext()
clientConn, cleanup, err := connectClient(cli, false)
Expand Down
11 changes: 11 additions & 0 deletions docs/release-notes/release-notes-0.14.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@
quotes](https://github.com/lightninglabs/lightning-terminal/pull/920).
Fixes fee estimation bug when using Loop In for a specific channel.

* [Fix inconsistent timestamp local such that we strictly use UTC
time](https://github.com/lightninglabs/lightning-terminal/pull/976).
This will mean that previously serialised timestamps that were not first
converted to UTC will not be interpreted as UTC and so may be off by a few
hours depending on the timezone of where the server is running. This should
not be an issue for majority of cases. The possible side effects are that an
LNC session's expiry may differ by a few hours. There is an unlikely edge case
that can happen if an LNC session is created, not connected to, and then LiT
is upgraded all within a 10 min timespan. If this happens then the session may
immediately be revoked. The solution to this is just to recreate the session.

### Functional Changes/Additions

### Technical and Architectural Updates
Expand Down
2 changes: 1 addition & 1 deletion firewall/request_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (r *RequestLogger) addNewAction(ri *RequestInfo,

action := &firewalldb.Action{
RPCMethod: ri.URI,
AttemptedAt: time.Now(),
AttemptedAt: time.Now().UTC(),
State: firewalldb.ActionStateInit,
}

Expand Down
2 changes: 1 addition & 1 deletion rules/rate_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (r *RateLimitEnforcer) HandleRequest(ctx context.Context, uri string,
// Determine the start time of the actions window.
startTime := time.Now().Add(
-time.Duration(rateLim.NumHours) * time.Hour,
)
).UTC()
Comment on lines 135 to +137
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be:

	startTime := time.Now().UTC().Add(
		-time.Duration(rateLim.NumHours) * time.Hour,
	)

Else there can be an discrepancy when the local timezone conversion happens right at a boundary of DST.


// Now count all relevant actions which have taken place after the
// start time.
Expand Down
14 changes: 7 additions & 7 deletions session_rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (s *sessionRpcServer) start(ctx context.Context) error {
continue
}

if sess.Expiry.Before(time.Now()) {
if sess.Expiry.Before(time.Now().UTC()) {
continue
}

Expand Down Expand Up @@ -190,7 +190,7 @@ func (s *sessionRpcServer) AddSession(ctx context.Context,
req *litrpc.AddSessionRequest) (*litrpc.AddSessionResponse, error) {

expiry := time.Unix(int64(req.ExpiryTimestampSeconds), 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one should also be:

Suggested change
expiry := time.Unix(int64(req.ExpiryTimestampSeconds), 0)
expiry := time.Unix(int64(req.ExpiryTimestampSeconds), 0).UTC()

Or we need to update protos to declare that the timestamp should explicitly be sent in through the request in UTC?

if time.Now().After(expiry) {
if time.Now().UTC().After(expiry) {
return nil, fmt.Errorf("expiry must be in the future")
}

Expand Down Expand Up @@ -364,7 +364,7 @@ func (s *sessionRpcServer) resumeSession(ctx context.Context,
}

// Don't resume an expired session.
if sess.Expiry.Before(time.Now()) {
if sess.Expiry.Before(time.Now().UTC()) {
log.Debugf("Not resuming session %x with expiry %s",
pubKeyBytes, sess.Expiry)

Expand Down Expand Up @@ -440,15 +440,15 @@ func (s *sessionRpcServer) resumeSession(ctx context.Context,
// we do not yet have a static remote pub key for.
if sess.RemotePublicKey == nil {
deadline := sess.CreatedAt.Add(s.cfg.firstConnectionDeadline)
if deadline.Before(time.Now()) {
if deadline.Before(time.Now().UTC()) {
log.Debugf("Deadline for session %x has already "+
"passed. Revoking session", pubKeyBytes)

return s.cfg.db.RevokeSession(pubKey)
}

// Start the deadline timer.
deadlineDuration := time.Until(deadline)
deadlineDuration := deadline.Sub(time.Now().UTC())
deadlineTimer := time.AfterFunc(deadlineDuration, func() {
close(firstConnTimout)
})
Expand Down Expand Up @@ -489,7 +489,7 @@ func (s *sessionRpcServer) resumeSession(ctx context.Context,
go func() {
defer s.wg.Done()

ticker := time.NewTimer(time.Until(sess.Expiry))
ticker := time.NewTimer(sess.Expiry.Sub(time.Now().UTC()))
defer ticker.Stop()

select {
Expand Down Expand Up @@ -839,7 +839,7 @@ func (s *sessionRpcServer) AddAutopilotSession(ctx context.Context,
}

expiry := time.Unix(int64(req.ExpiryTimestampSeconds), 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

if time.Now().After(expiry) {
if time.Now().UTC().After(expiry) {
return nil, fmt.Errorf("expiry must be in the future")
}

Expand Down
Loading