-
Notifications
You must be signed in to change notification settings - Fork 32
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
update dest tx hash before status/improve obversability #2888
Changes from 5 commits
20bda71
160a5b2
dff84eb
690b167
2358e16
72dbf2b
9f409ed
4951dc7
312e935
53067c3
5504b32
35baeea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
"go.opentelemetry.io/otel/trace" | ||
) | ||
|
||
var maxRPCRetryTime = 15 * time.Second | ||
var maxRPCRetryTime = 30 * time.Second | ||
|
||
// handleBridgeRequestedLog handles the BridgeRequestedLog event. | ||
// Step 1: Seen | ||
|
@@ -71,7 +71,16 @@ | |
} | ||
|
||
var bridgeTx fastbridge.IFastBridgeBridgeTransaction | ||
call := func(ctx context.Context) error { | ||
call := func(parentCtx context.Context) error { | ||
trajan0x marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ctx, newSpan := r.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes( | ||
attribute.String("transaction_id", hexutil.Encode(req.TransactionId[:])), | ||
attribute.String("bridge_address", req.Raw.Address.String()), | ||
)) | ||
|
||
defer func() { | ||
metrics.EndSpanWithErr(newSpan, err) | ||
}() | ||
|
||
bridgeTx, err = fastBridge.GetBridgeTransaction(&bind.CallOpts{Context: ctx}, req.Request) | ||
if err != nil { | ||
return fmt.Errorf("could not get bridge transaction: %w", err) | ||
|
@@ -332,14 +341,15 @@ | |
} | ||
|
||
// TODO: this can still get re-orged | ||
err = r.db.UpdateQuoteRequestStatus(ctx, req.TransactionId, reldb.RelayCompleted, nil) | ||
if err != nil { | ||
return fmt.Errorf("could not update request status: %w", err) | ||
} | ||
err = r.db.UpdateDestTxHash(ctx, req.TransactionId, req.Raw.TxHash) | ||
if err != nil { | ||
return fmt.Errorf("could not update dest tx hash: %w", err) | ||
} | ||
|
||
err = r.db.UpdateQuoteRequestStatus(ctx, req.TransactionId, reldb.RelayCompleted, nil) | ||
if err != nil { | ||
return fmt.Errorf("could not update request status: %w", err) | ||
} | ||
trajan0x marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return nil | ||
} | ||
|
||
|
@@ -421,6 +431,14 @@ | |
|
||
var canClaim bool | ||
claimCall := func(ctx context.Context) error { | ||
ctx, newSpan := q.metrics.Tracer().Start(ctx, "checkClaimCheck", trace.WithAttributes( | ||
attribute.String("transaction_id", hexutil.Encode(request.TransactionID[:])), | ||
)) | ||
|
||
defer func() { | ||
metrics.EndSpanWithErr(newSpan, err) | ||
}() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhanced tracing and error handling. The function has been enhanced with tracing and error handling for metrics, improving observability and reliability. The added lines are not covered by tests. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
Enhanced tracing and error handling. The The added lines are not covered by tests. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
|
||
|
||
canClaim, err = q.Origin.Bridge.CanClaim(&bind.CallOpts{Context: ctx}, request.TransactionID, q.RelayerAddress) | ||
if err != nil { | ||
return fmt.Errorf("could not check if can claim: %w", err) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid reassigning
ctx
within thecall
function.Reassigning
ctx
within thecall
function can lead to potential issues. Instead, use a different variable name for the new context.Committable suggestion
Tools
GitHub Check: Lint (services/rfq)