Skip to content

Commit

Permalink
fix(produce loop): handle unauthenticated error in settlement layer (#…
Browse files Browse the repository at this point in the history
…726)

Co-authored-by: github-actions <github-actions@github.com>
  • Loading branch information
2 people authored and omritoptix committed Apr 28, 2024
1 parent eb6c27d commit 48ff85f
Show file tree
Hide file tree
Showing 10 changed files with 279 additions and 47 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# [](https://github.com/dymensionxyz/dymint/compare/v1.1.0-rc02...v) (2024-04-26)
# [](https://github.com/dymensionxyz/dymint/compare/v1.1.0-rc02...v) (2024-04-27)


Check failure on line 3 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / markdownlint

Multiple consecutive blank lines [Expected: 1; Actual: 2]
### Bug Fixes

Check failure on line 4 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / markdownlint

Heading levels should only increment by one level at a time [Expected: h2; Actual: h3]

* **p2p:** validate block before applying and not before caching in p2p gossiping ([#723](https://github.com/dymensionxyz/dymint/issues/723)) ([98371b5](https://github.com/dymensionxyz/dymint/commit/98371b5220613e70f3274fab5593e02ba532f7db))


Check failure on line 8 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / markdownlint

Multiple consecutive blank lines [Expected: 1; Actual: 2]

Check failure on line 9 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / markdownlint

Multiple consecutive blank lines [Expected: 1; Actual: 3]
Expand Down
6 changes: 2 additions & 4 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ type Manager struct {
// Data retrieval
Retriever da.BatchRetriever

// Synchronization
SyncTargetDiode diodes.Diode

SyncTarget atomic.Uint64
SyncTarget atomic.Uint64

// Block production
shouldProduceBlocksCh chan bool
Expand Down Expand Up @@ -83,9 +81,9 @@ type Manager struct {
// pendingBatch is the result of the last DA submission
// that is pending settlement layer submission.
// It is used to avoid double submission of the same batch.
// It's protected by submitBatchMutex.
pendingBatch *PendingBatch

// Logging
logger types.Logger

// Cached blocks and commits for applying at future heights. Invariant: the block and commit are .Valid() (validated sigs etc)
Expand Down
67 changes: 34 additions & 33 deletions block/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ package block

import (
"context"
"errors"
"fmt"
"time"

"github.com/dymensionxyz/dymint/gerr"

"github.com/dymensionxyz/dymint/da"
"github.com/dymensionxyz/dymint/types"
)
Expand All @@ -21,63 +24,70 @@ func (m *Manager) SubmitLoop(ctx context.Context) {
return
// trigger by time
case <-ticker.C:
m.HandleSubmissionTrigger(ctx)
err := m.HandleSubmissionTrigger(ctx)
if errors.Is(err, gerr.ErrAborted) {
continue
}
if errors.Is(err, gerr.ErrUnauthenticated) {
panic(fmt.Errorf("handle submission trigger: %w", err))
}
if err != nil {
m.logger.Error("handle submission trigger", "error", err)
}
}
}
}

// handleSubmissionTrigger processes the submission trigger event. It checks if there are new blocks produced since the last submission.
// HandleSubmissionTrigger processes the submission trigger event. It checks if there are new blocks produced since the last submission.
// If there are, it attempts to submit a batch of blocks. It then attempts to produce an empty block to ensure IBC messages
// pass through during the batch submission process due to proofs requires for ibc messages only exist on the next block.
// Finally, it submits the next batch of blocks and updates the sync target to the height of
// the last block in the submitted batch.
func (m *Manager) HandleSubmissionTrigger(ctx context.Context) {
if !m.submitBatchMutex.TryLock() { // Attempt to lock for batch processing
m.logger.Debug("Batch submission already in process, skipping submission")
return
func (m *Manager) HandleSubmissionTrigger(ctx context.Context) error {
if !m.submitBatchMutex.TryLock() {
return fmt.Errorf("batch submission already in process, skipping submission: %w", gerr.ErrAborted)
}
defer m.submitBatchMutex.Unlock() // Ensure unlocking at the end

// Load current sync target and height to determine if new blocks are available for submission.
syncTarget, height := m.SyncTarget.Load(), m.Store.Height()
if height <= syncTarget { // Check if there are new blocks since last sync target.
return // Exit if no new blocks are produced.
return nil // Exit if no new blocks are produced.
}
// We try and produce an empty block to make sure relevant ibc messages will pass through during the batch submission: https://github.com/dymensionxyz/research/issues/173.
err := m.ProduceAndGossipBlock(ctx, true)
if err != nil {
m.logger.Error("produce empty block", "error", err)
m.logger.Error("produce and gossip empty block", "error", err)
}

if m.pendingBatch != nil {
m.logger.Info("pending batch exists", "startHeight", m.pendingBatch.batch.StartHeight, "endHeight", m.pendingBatch.batch.EndHeight)
} else {
if m.pendingBatch == nil {
nextBatch, err := m.createNextBatch()
if err != nil {
m.logger.Error("get next batch", "error", err)
return
return fmt.Errorf("create next batch: %w", err)
}

resultSubmitToDA, err := m.submitNextBatchToDA(nextBatch)
if err != nil {
m.logger.Error("submit next batch", "error", err)
return
return fmt.Errorf("submit next batch to da: %w", err)
}

m.pendingBatch = &PendingBatch{
daResult: resultSubmitToDA,
batch: nextBatch,
}
} else {
m.logger.Info("pending batch already exists", "startHeight", m.pendingBatch.batch.StartHeight, "endHeight", m.pendingBatch.batch.EndHeight)
}

syncHeight, err := m.submitPendingBatchToSL()
syncHeight, err := m.submitPendingBatchToSL(*m.pendingBatch)
if err != nil {
m.logger.Error("submit next batch to SL", "error", err)
return
return fmt.Errorf("submit pending batch to sl: %w", err)
}
m.pendingBatch = nil

// Update the syncTarget to the height of the last block in the last batch as seen by this node.
m.UpdateSyncParams(syncHeight)
return nil
}

func (m *Manager) createNextBatch() (*types.Batch, error) {
Expand Down Expand Up @@ -106,7 +116,7 @@ func (m *Manager) submitNextBatchToDA(nextBatch *types.Batch) (*da.ResultSubmitB
m.logger.Error("validate last block in batch is empty", "startHeight", startHeight, "endHeight", actualEndHeight, "error", err)
return nil, err
}
// Verify the last block in the batch is an empty block and that no ibc messages has accidentially passed through.
// Verify the last block in the batch is an empty block and that no ibc messages has accidentally passed through.
// This block may not be empty if another block has passed it in line. If that's the case our empty block request will
// be sent to the next batch.
if !isLastBlockEmpty {
Expand All @@ -124,23 +134,14 @@ func (m *Manager) submitNextBatchToDA(nextBatch *types.Batch) (*da.ResultSubmitB
return &resultSubmitToDA, nil
}

func (m *Manager) submitPendingBatchToSL() (uint64, error) {
if m.pendingBatch == nil {
return 0, fmt.Errorf("no pending batch to submit")
}

// Submit batch to SL
startHeight := m.pendingBatch.batch.StartHeight
actualEndHeight := m.pendingBatch.batch.EndHeight
err := m.SLClient.SubmitBatch(m.pendingBatch.batch, m.DAClient.GetClientType(), m.pendingBatch.daResult)
func (m *Manager) submitPendingBatchToSL(p PendingBatch) (uint64, error) {
startHeight := p.batch.StartHeight
actualEndHeight := p.batch.EndHeight
err := m.SLClient.SubmitBatch(p.batch, m.DAClient.GetClientType(), p.daResult)
if err != nil {
m.logger.Error("submit batch to SL", "startHeight", startHeight, "endHeight", actualEndHeight, "error", err)
return 0, err
return 0, fmt.Errorf("sl client submit batch: startheight: %d: actual end height: %d: %w", startHeight, actualEndHeight, err)
}

// Clear pending batch
m.pendingBatch = nil

return actualEndHeight, nil
}

Expand Down
15 changes: 15 additions & 0 deletions gerr/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Package gerr provides a systematic way to think about errors. It is based on google API design advice here
// https://cloud.google.com/apis/design/errors
// https://cloud.google.com/apis/design/errors#handling_errors
// https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto
// In particular, it's important to avoid defining additional errors. Rather, all important errors should wrap one of
// the errors defined in this package.
//
// "Google APIs must use the canonical error codes defined by google.rpc.Code. Individual APIs must avoid defining
// additional error codes, since developers are very unlikely to write logic to handle a large number of error codes.
// For reference, handling an average of three error codes per API call would mean most application logic would just
// be for error handling, which would not be a good developer experience."
//
// Note: this package can be extended to automatically return the correct GRPC/HTTP codes too, if needed.
// Note: this package could be lifted out and shared across more dymension code, to help us standardise.
package gerr
158 changes: 158 additions & 0 deletions gerr/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
package gerr

import "errors"

/*
See https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto
See also package doc
*/

// ErrCancelled : The operation was cancelled, typically by the caller.
//
// HTTP Mapping: 499 Client Closed Request
var ErrCancelled = errors.New("cancelled") // CANCELLED

// ErrUnknown : Unknown error. For example, this error may be returned when
// a `Status` value received from another address space belongs to
// an error space that is not known in this address space. Also
// errors raised by APIs that do not return enough error information
// may be converted to this error.
//
// HTTP Mapping: 500 Internal Server Error
var ErrUnknown = errors.New("unknown") // UNKNOWN

// ErrInvalidArgument : The client specified an invalid argument. Note that this differs
// from `FAILED_PRECONDITION`. `INVALID_ARGUMENT` indicates arguments
// that are problematic regardless of the state of the system
// (e.g., a malformed file name).
//
// HTTP Mapping: 400 Bad Request
var ErrInvalidArgument = errors.New("invalid argument") // INVALID_ARGUMENT

// ErrDeadlineExceeded : The deadline expired before the operation could complete. For operations
// that change the state of the system, this error may be returned
// even if the operation has completed successfully. For example, a
// successful response from a server could have been delayed long
// enough for the deadline to expire.
//
// HTTP Mapping: 504 Gateway Timeout
var ErrDeadlineExceeded = errors.New("deadline exceeded") // DEADLINE_EXCEEDED

// ErrNotFound : Some requested entity (e.g., file or directory) was not found.
//
// Note to server developers: if a request is denied for an entire class
// of users, such as gradual feature rollout or undocumented allowlist,
// `NOT_FOUND` may be used. If a request is denied for some users within
// a class of users, such as user-based access control, `PERMISSION_DENIED`
// must be used.
//
// HTTP Mapping: 404 Not Found
var ErrNotFound = errors.New("not found") // NOT_FOUND

// ErrAlreadyExist : The entity that a client attempted to create (e.g., file or directory)
// already exists.
//
// HTTP Mapping: 409 Conflict
var ErrAlreadyExist = errors.New("already exist") // ALREADY_EXIST

// ErrPermissionDenied : The caller does not have permission to execute the specified
// operation. `PERMISSION_DENIED` must not be used for rejections
// caused by exhausting some resource (use `RESOURCE_EXHAUSTED`
// instead for those errors). `PERMISSION_DENIED` must not be
// used if the caller can not be identified (use `UNAUTHENTICATED`
// instead for those errors). This error code does not imply the
// request is valid or the requested entity exists or satisfies
// other pre-conditions.
//
// HTTP Mapping: 403 Forbidden
var ErrPermissionDenied = errors.New("permission denied") // PERMISSION_DENIED

// ErrUnauthenticated : The request does not have valid authentication credentials for the
// operation.
//
// HTTP Mapping: 401 Unauthorized
var ErrUnauthenticated = errors.New("unauthenticated") // UNAUTHENTICATED

// ErrResourceExhausted : Some resource has been exhausted, perhaps a per-user quota, or
// perhaps the entire file system is out of space.
//
// HTTP Mapping: 429 Too Many Requests
var ErrResourceExhausted = errors.New("resource exhausted") // RESOURCE_EXHAUSTED

// ErrFailedPrecondition : The operation was rejected because the system is not in a state
// required for the operation's execution. For example, the directory
// to be deleted is non-empty, an rmdir operation is applied to
// a non-directory, etc.
//
// Service implementors can use the following guidelines to decide
// between `FAILED_PRECONDITION`, `ABORTED`, and `UNAVAILABLE`:
//
// (a) Use `UNAVAILABLE` if the client can retry just the failing call.
// (b) Use `ABORTED` if the client should retry at a higher level. For
// example, when a client-specified test-and-set fails, indicating the
// client should restart a read-modify-write sequence.
// (c) Use `FAILED_PRECONDITION` if the client should not retry until
// the system state has been explicitly fixed. For example, if an "rmdir"
// fails because the directory is non-empty, `FAILED_PRECONDITION`
// should be returned since the client should not retry unless
// the files are deleted from the directory.
//
// HTTP Mapping: 400 Bad Request
var ErrFailedPrecondition = errors.New("failed precondition") // FAILED_PRECONDITION

// ErrAborted : The operation was aborted, typically due to a concurrency issue such as
// a sequencer check failure or transaction abort.
//
// See the guidelines above for deciding between `FAILED_PRECONDITION`,
// `ABORTED`, and `UNAVAILABLE`.
//
// HTTP Mapping: 409 Conflict
var ErrAborted = errors.New("aborted") // ABORTED

// ErrOutOfRange : The operation was attempted past the valid range. E.g., seeking or
// reading past end-of-file.
//
// Unlike `INVALID_ARGUMENT`, this error indicates a problem that may
// be fixed if the system state changes. For example, a 32-bit file
// system will generate `INVALID_ARGUMENT` if asked to read at an
// offset that is not in the range [0,2^32-1], but it will generate
// `OUT_OF_RANGE` if asked to read from an offset past the current
// file size.
//
// There is a fair bit of overlap between `FAILED_PRECONDITION` and
// `OUT_OF_RANGE`. We recommend using `OUT_OF_RANGE` (the more specific
// error) when it applies so that callers who are iterating through
// a space can easily look for an `OUT_OF_RANGE` error to detect when
// they are done.
//
// HTTP Mapping: 400 Bad Request
var ErrOutOfRange = errors.New("out of range") // OUT_OF_RANGE

// ErrUnimplemented : The operation is not implemented or is not supported/enabled in this
// service.
//
// HTTP Mapping: 501 Not Implemented
var ErrUnimplemented = errors.New("unimplemented") // UNIMPLEMENTED

// ErrInternal : Internal errors. This means that some invariants expected by the
// underlying system have been broken. This error code is reserved
// for serious errors.
//
// HTTP Mapping: 500 Internal Server Error
var ErrInternal = errors.New("internal") // INTERNAL

// ErrUnavailable : The service is currently unavailable. This is most likely a
// transient condition, which can be corrected by retrying with
// a backoff. Note that it is not always safe to retry
// non-idempotent operations.
//
// See the guidelines above for deciding between `FAILED_PRECONDITION`,
// `ABORTED`, and `UNAVAILABLE`.
//
// HTTP Mapping: 503 Service Unavailable
var ErrUnavailable = errors.New("unavailable") // UNAVAILABLE

// ErrDataLoss : Unrecoverable data loss or corruption.
//
// HTTP Mapping: 500 Internal Server Error
var ErrDataLoss = errors.New("data loss") // DATA_LOSS
17 changes: 16 additions & 1 deletion settlement/dymension/cosmosclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ package dymension

import (
"context"
"errors"
"fmt"
"strings"

"github.com/dymensionxyz/dymint/gerr"

sdkclient "github.com/cosmos/cosmos-sdk/client"
sdktypes "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -64,5 +69,15 @@ func (c *cosmosClient) GetSequencerClient() sequencertypes.QueryClient {
}

func (c *cosmosClient) GetAccount(accountName string) (cosmosaccount.Account, error) {
return c.AccountRegistry.GetByName(accountName)
acc, err := c.AccountRegistry.GetByName(accountName)
if err != nil {
if strings.Contains(err.Error(), "too many failed passphrase attempts") {
return cosmosaccount.Account{}, fmt.Errorf("account registry get by name: %w:%w", gerr.ErrUnauthenticated, err)
}
var accNotExistErr *cosmosaccount.AccountDoesNotExistError
if errors.As(err, &accNotExistErr) {
return cosmosaccount.Account{}, fmt.Errorf("account registry get by name: %w:%w", gerr.ErrNotFound, err)
}
}
return acc, err
}
Loading

0 comments on commit 48ff85f

Please sign in to comment.