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 28ae482
Showing 1 changed file with 18 additions and 15 deletions.
33 changes: 18 additions & 15 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,24 +672,27 @@ func (wfe *WebFrontEndImpl) checkNewAccountLimits(ctx context.Context, ip net.IP
}

// refundNewAccountLimits is typically called when a new account creation fails.
// 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) {
// It waits for limitCheck channel to close and then 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, limitCheck <-chan struct{}, ip net.IP) {
if wfe.limiter == nil && wfe.txnBuilder == nil {
// Limiter is disabled.
return
}

warn := func(err error, limit ratelimits.Name) {
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
return
}
// TODO(#5545): Once key-value rate limits are authoritative this log
// line should be removed in favor of returning the error.
// Log the error for debugging purposes.
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 +845,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 28ae482

Please sign in to comment.