-
Notifications
You must be signed in to change notification settings - Fork 252
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
Replay mechanism for QGB relayer #344
Conversation
Codecov Report
@@ Coverage Diff @@
## qgb-integration #344 +/- ##
==================================================
Coverage ? 13.36%
==================================================
Files ? 54
Lines ? 9724
Branches ? 0
==================================================
Hits ? 1300
Misses ? 8333
Partials ? 91 Continue to review full report at Codecov.
|
Can be merged after: #342 |
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.
I think this makes sense. I have a question or two, but let's sync on what we want to do next sometime soon. Pre-approving until then
// TODO make gas limit configurable | ||
builder.SetGasLimit(9999999999999) | ||
// TODO: update this api | ||
// via https://github.com/celestiaorg/celestia-app/pull/187/commits/37f96d9af30011736a3e6048bbb35bad6f5b795c | ||
tx, err := bc.signer.BuildSignedTx(builder, msg) | ||
if err != nil { | ||
return "", 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.
are we planning on handling this before we merge this, in a different refactor, or do we want to create an issue?
as suggested here, we might be able to using something similar to
celestia-app/x/payment/types/wirepayformessage.go
Lines 50 to 52 in 9f15b0a
for _, option := range options { | |
builder = option(builder) | |
} |
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.
I think this can be addressed in a separate PR as this PR only focuses on replaying missing attestations to the QGB contract
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.
@evan-forbes Can you please create an issue for this as I am not sure how to use these options.
Thanks a lot
* adds valset request by nonce * add querier, broadcaster and updates orchestrator to use them * partially fix the tests for the new design * update the deployer for the new design * update orchestrator to new design * Adds querier and evm client update * update relayer to new design * fix query.proto * update deploy_command to use new Querier * formatting * formatting * go.sum * revert go.mod change
This change will make the relayer start from the QGB contract last state and relay from there.
Closes: #338 and #345