Skip to content

Commit

Permalink
Address comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
beautifulentropy committed Jan 16, 2024
1 parent e61140e commit 7ad61fd
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ func (wfe *WebFrontEndImpl) checkNewAccountLimits(ctx context.Context, ip net.IP
// It refunds the limit quota consumed by the request, allowing the caller to
// retry immediately. If an error is encountered during the refund, it is logged
// but not returned.
func (wfe *WebFrontEndImpl) refundNewAccountLimits(ctx context.Context, ip net.IP) {
func (wfe *WebFrontEndImpl) refundNewAccountLimits(ctx context.Context, limitCheck <-chan struct{}, ip net.IP) {
if wfe.limiter == nil && wfe.txnBuilder == nil {
// Limiter is disabled.
return
Expand All @@ -690,6 +690,13 @@ func (wfe *WebFrontEndImpl) refundNewAccountLimits(ctx context.Context, ip net.I
wfe.log.Warningf("refunding %s rate limit: %s", limit, err)
}

select {
case <-ctx.Done():
return
case <-limitCheck:
// Continue with refunding the limits.
}

txn, err := wfe.txnBuilder.RegistrationsPerIPAddressTransaction(ip)
if err != nil {
warn(err, ratelimits.NewRegistrationsPerIPAddress)
Expand Down Expand Up @@ -842,29 +849,29 @@ func (wfe *WebFrontEndImpl) NewAccount(

// TODO(#5545): Spending and Refunding can be async until these rate limits
// are authoritative. This saves us from adding latency to each request.
doneCheckingLimits := make(chan struct{})
limitCheck := make(chan struct{})
var newRegistrationSuccessful bool
var errIsRateLimit bool

go func() {
// Close the channel on goroutine completion.
defer close(doneCheckingLimits)
defer close(limitCheck)
wfe.checkNewAccountLimits(ctx, ip)
}()

defer func() {
if !newRegistrationSuccessful && !errIsRateLimit {
// Wait for the rate limit check to complete before attempting to refund
// the limits. If the check failed, we don't want to refund anything.
<-doneCheckingLimits
go wfe.refundNewAccountLimits(ctx, ip)
go wfe.refundNewAccountLimits(ctx, limitCheck, ip)
}
}()

// Send the registration to the RA via grpc
acctPB, err := wfe.ra.NewRegistration(ctx, &reg)
if err != nil {
if errors.Is(err, berrors.RateLimit) {
// Request was denied by a legacy rate limit. In this error case we
// do not want to refund the quota consumed by the request because
// repeated requests would result in unearned refunds.
errIsRateLimit = true
}
if errors.Is(err, berrors.Duplicate) {
Expand Down

0 comments on commit 7ad61fd

Please sign in to comment.