Skip to content
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

Exponential backoff in logic call skips, frequent Ethereum height updates #400

Merged
merged 20 commits into from
May 5, 2022

Conversation

EricBolten
Copy link
Contributor

@EricBolten EricBolten commented Apr 22, 2022

  • Rather than skipping calls forever after one failure, implement an exponential backoff scheme to allow for retries against transient Ethereum error conditions. Much like the previous version, this is in-memory only and the backoff process will reset on process restart.
  • Add the ability to permanently skip calls in addition to having the backoff option.
  • Add code to parse Gravity-defined error types, and use them to handle permanent skip conditions (yes, this code is pretty ugly, but functional).
  • Add code to the gravity module and the orchestrator to periodically submit observed Ethereum heights, so we don't have to wait for a bridge event to let the Cosmos chain know the current Ethereum height. It chooses a height by a consensus-weighted vote on acceptable heights (e.g. equal to or lower than the observed height of each validator).
  • Add a query endpoint so you can use the CLI to look up what the last observed heights are.

@EricBolten EricBolten requested review from mvid and cbrit April 22, 2022 23:59
@EricBolten EricBolten temporarily deployed to CI April 23, 2022 00:06 Inactive
@cbrit
Copy link
Member

cbrit commented Apr 23, 2022

LGTM

@EricBolten EricBolten changed the title Exponential backoff in logic call skips Exponential backoff in logic call skips, frequent Ethereum height updates Apr 28, 2022
@EricBolten EricBolten temporarily deployed to CI April 28, 2022 05:21 Inactive
_ => None
}
}
_ => None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see what you meant about matching

@EricBolten EricBolten temporarily deployed to CI May 3, 2022 15:16 Inactive
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2022

Codecov Report

Merging #400 (0ca9c09) into main (5c26884) will increase coverage by 0.42%.
The diff coverage is 46.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   28.26%   28.68%   +0.42%     
==========================================
  Files          47       47              
  Lines        6401     6550     +149     
==========================================
+ Hits         1809     1879      +70     
- Misses       4447     4524      +77     
- Partials      145      147       +2     
Impacted Files Coverage Δ
module/x/gravity/client/cli/query.go 0.00% <0.00%> (ø)
module/x/gravity/handler.go 43.24% <0.00%> (-3.82%) ⬇️
module/x/gravity/keeper/ethereum_event_vote.go 50.73% <0.00%> (-0.51%) ⬇️
module/x/gravity/keeper/grpc_query.go 17.17% <0.00%> (-0.44%) ⬇️
module/x/gravity/types/codec.go 5.71% <0.00%> (-0.06%) ⬇️
module/x/gravity/types/key.go 0.00% <0.00%> (ø)
module/x/gravity/types/msgs.go 12.00% <0.00%> (-1.73%) ⬇️
module/x/gravity/keeper/keeper.go 86.27% <45.71%> (-3.60%) ⬇️
module/x/gravity/keeper/msg_server.go 61.50% <70.00%> (+0.35%) ⬆️
module/x/gravity/abci.go 78.85% <100.00%> (+4.28%) ⬆️

@EricBolten EricBolten temporarily deployed to CI May 3, 2022 15:31 Inactive
@EricBolten EricBolten temporarily deployed to CI May 3, 2022 16:17 Inactive
@EricBolten EricBolten temporarily deployed to CI May 3, 2022 19:14 Inactive
@@ -161,7 +161,8 @@ func eventVoteRecordTally(ctx sdk.Context, k keeper.Keeper) {
// 2. The proposed consensus heights from this process are greater than the values stored from the last time
// we observed an Ethereum event from the bridge
func updateObservedEthereumHeight(ctx sdk.Context, k keeper.Keeper) {
if ctx.BlockHeight()%10 != 0 {
// wait some minutes before checking the height votes
if ctx.BlockHeight()%50 != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a param?

@EricBolten EricBolten merged commit c5d8fd8 into main May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants