-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(rfq-relayer): wait for finality before proving #3062
Changes from 7 commits
d3e88af
c23ae24
f00f3c8
7036c04
97df029
6842788
a5286c8
212d9b8
f45dad8
108c64d
12715d9
3524d13
19dd299
99e5d50
6d41b64
978bcf2
65a06e0
db7f9fd
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 | ||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -215,6 +215,20 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
return value, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
// GetFinalityConfirmations returns the FinalityConfirmations for the given chainID. | ||||||||||||||||||||||||||||||||||||||||||||||||||
func (c Config) GetFinalityConfirmations(chainID int) (value uint64, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
rawValue, err := c.getChainConfigValue(chainID, "FinalityConfirmations") | ||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
return value, err | ||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
value, ok := rawValue.(uint64) | ||||||||||||||||||||||||||||||||||||||||||||||||||
if !ok { | ||||||||||||||||||||||||||||||||||||||||||||||||||
return value, fmt.Errorf("failed to cast FinalityConfirmations to int") | ||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||
return value, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+219
to
+230
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. Add unit tests for the new function. The function should include unit tests to ensure its correctness. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
Correct the error message in the type assertion failure. The error message should mention - return value, fmt.Errorf("failed to cast FinalityConfirmations to int")
+ return value, fmt.Errorf("failed to cast FinalityConfirmations to uint64") Committable suggestion
Suggested change
ToolsGitHub Check: codecov/patch
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
// GetNativeToken returns the NativeToken for the given chainID. | ||||||||||||||||||||||||||||||||||||||||||||||||||
func (c Config) GetNativeToken(chainID int) (value string, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
rawValue, err := c.getChainConfigValue(chainID, "NativeToken") | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -378,8 +378,28 @@ | |
// Step 6: ProvePosting | ||
// | ||
// This is the sixth step in the bridge process. Here we submit the claim transaction to the origin chain. | ||
func (q *QuoteRequestHandler) handleRelayCompleted(ctx context.Context, _ trace.Span, request reldb.QuoteRequest) (err error) { | ||
// relays been completed, it's time to go back to the origin chain and try to prove | ||
func (q *QuoteRequestHandler) handleRelayCompleted(ctx context.Context, span trace.Span, request reldb.QuoteRequest) (err error) { | ||
// first, ensure that enough confirmations have passed before proving | ||
receipt, err := q.Dest.Client.TransactionReceipt(ctx, request.DestTxHash) | ||
if err != nil { | ||
return fmt.Errorf("could not get transaction receipt: %w", err) | ||
} | ||
currentBlockNumber := q.Origin.LatestBlock() | ||
proveConfirmations, err := q.cfg.GetFinalityConfirmations(int(q.Dest.ChainID)) | ||
if err != nil { | ||
return fmt.Errorf("could not get prove confirmations: %w", err) | ||
} | ||
span.SetAttributes( | ||
attribute.Int("current_block_number", int(currentBlockNumber)), | ||
attribute.Int("dest_block_number", int(receipt.BlockNumber.Int64())), | ||
attribute.Int("prove_confirmations", int(proveConfirmations)), | ||
) | ||
if currentBlockNumber < request.BlockNumber+proveConfirmations { | ||
span.AddEvent("not enough confirmations") | ||
return nil | ||
} | ||
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. Reminder: Add tests. The added lines are not covered by tests. Ensure that the new logic is adequately tested. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
|
||
|
||
// relay has been finalized, it's time to go back to the origin chain and try to prove | ||
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. @dwasse we need to do a check similar to Guard's In the event where we submitted a tx, witnessed a The current code will still get a valid receipt for |
||
_, err = q.Origin.SubmitTransaction(ctx, func(transactor *bind.TransactOpts) (tx *types.Transaction, err error) { | ||
tx, err = q.Origin.Bridge.Prove(transactor, request.RawRequest, request.DestTxHash) | ||
if err != nil { | ||
|
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.
Fix potential nil pointer dereference.
The function
isFinalized
has a potential issue with nil pointer dereference. Ensure that thereceipt
is checked for nil before accessing its fields.Apply this diff to fix the potential nil pointer dereference:
Committable suggestion
Tools
GitHub Check: Lint (services/rfq)