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

core,pm: handle zero/nil gas price from gas price monitor #1830

Merged
merged 3 commits into from
Apr 8, 2021

Conversation

kyriediculous
Copy link
Contributor

@kyriediculous kyriediculous commented Apr 7, 2021

What does this pull request do? Explain your changes. (required)
When the gas price monitor returns a gas price of 0 or nil in on-chain mode the node will crash with a division-by-zero error. This bug caused a majority of the orchestrators on the public network to all crash simultaneously when a common hosted ethereum provider, infura, could have potentially returned a gas price of 0. (see below).

Specific updates (required)

  • In recipient.txCost() if the gasPriceMonitor returns 0 (or nil , but this should be impossible due to the above update) txCost will return 0
  • In recipient.TxCostMultiplier() if the value returned from recipient.txCost() is 0 , return big.NewRat(0,1)
  • In orchestrator.priceInfo() if the txCostMultiplier return from recipient.TxCostMultiplier() is 0/1 use a price multiplier of 1 (no overhead charged because of no tx cost).

How did you test each of these updates (required)

  • Added unit tests
  • Manual test on private network with 0 gas price

Motivation
Decided to add extra checks to deal with cases of nil and 0 values down the call chain. While this is a more elaborate fix it would prevent breaking the software on private networks with a gas price of 0 or running against an ethereum node that also mines with a gas price of 0.

Checklist:

Context

panic: division by zero

goroutine 336441272 [running]:
math/big.(*Rat).SetFrac(0xc003369480, 0xc00019aea0, 0xc000ccfb20, 0x1ee6364f)
        /usr/local/go/src/math/big/rat.go:307 +0x465
github.com/livepeer/go-livepeer/pm.(*recipient).TxCostMultiplier(0xc0001780c0, 0x7d1b06f7c8c4c7c3, 0xfe5953ee6e76726a, 0xc01ee6364f, 0x8361c8, 0xc001bee460, 0xc001bee440)
        /root/go-livepool/pm/recipient.go:289 +0xce
github.com/livepeer/go-livepeer/core.(*orchestrator).priceInfo(0xc000512910, 0x7d1b06f7c8c4c7c3, 0xfe5953ee6e76726a, 0x1ee6364f, 0x0, 0xc001156a20, 0xc0023b58b0)
        /root/go-livepool/core/orchestrator.go:283 +0x22d
github.com/livepeer/go-livepeer/core.(*orchestrator).PriceInfo(0xc000512910, 0x7d1b06f7c8c4c7c3, 0xfe5953ee6e76726a, 0x1ee6364f, 0x0, 0x20, 0xc001bee4c0)
        /root/go-livepool/core/orchestrator.go:261 +0x96
github.com/livepeer/go-livepeer/server.getPriceInfo(0x1fccc58, 0xc000512910, 0x7d1b06f7c8c4c7c3, 0xfe5953ee6e76726a, 0xc01ee6364f, 0xc0023b5988, 0x690ba7, 0x0)
        /root/go-livepool/server/rpc.go:282 +0x53
github.com/livepeer/go-livepeer/server.orchestratorInfo(0x1fccc58, 0xc000512910, 0x7d1b06f7c8c4c7c3, 0xfe5953ee6e76726a, 0x1ee6364f, 0xc001bee4c0, 0x1c, 0x50, 0x0, 0x0)
        /root/go-livepool/server/rpc.go:286 +0x6a
github.com/livepeer/go-livepeer/server.getOrchestrator(0x1fccc58, 0xc000512910, 0xc0042c35e0, 0x1c51840, 0xc004ed9600, 0x1fad5d0)
        /root/go-livepool/server/rpc.go:272 +0x305
github.com/livepeer/go-livepeer/server.(*lphttp).GetOrchestrator(0xc0001418e0, 0x1fbc0f0, 0xc004ed9680, 0xc0042c35e0, 0xc0001418e0, 0xc004ed9680, 0xc000067b88)
        /root/go-livepool/server/rpc.go:140 +0x45
github.com/livepeer/go-livepeer/net._Orchestrator_GetOrchestrator_Handler(0x1c51840, 0xc0001418e0, 0x1fbc0f0, 0xc004ed9680, 0xc0050104e0, 0x0, 0x1fbc0f0, 0xc004ed9680, 0xc0067d9860, 0x59)
        /root/go-livepool/net/lp_rpc.pb.go:1688 +0x214
google.golang.org/grpc.(*Server).processUnaryRPC(0xc00011aea0, 0x1fc7658, 0xc003d01320, 0xc00ffa0a00, 0xc00011cf00, 0x2a60960, 0x0, 0x0, 0x0)
        /root/go/pkg/mod/google.golang.org/grpc@v1.28.0/server.go:1082 +0x52b
google.golang.org/grpc.(*Server).handleStream(0xc00011aea0, 0x1fc7658, 0xc003d01320, 0xc00ffa0a00, 0x0)
        /root/go/pkg/mod/google.golang.org/grpc@v1.28.0/server.go:1405 +0xccf
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc000500a20, 0xc00011aea0, 0x1fc7658, 0xc003d01320, 0xc00ffa0a00)
        /root/go/pkg/mod/google.golang.org/grpc@v1.28.0/server.go:746 +0xab
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /root/go/pkg/mod/google.golang.org/grpc@v1.28.0/server.go:744 +0xa5
       ```

@kyriediculous kyriediculous requested a review from yondonfu April 7, 2021 17:46
@kyriediculous kyriediculous marked this pull request as ready for review April 7, 2021 19:21
core/orchestrator.go Outdated Show resolved Hide resolved
pm/recipient.go Outdated Show resolved Hide resolved
eth/gaspricemonitor.go Outdated Show resolved Hide resolved
@kyriediculous kyriediculous changed the title core,eth,pm: handle zero/nil gas price from gas price monitor core,pm: handle zero/nil gas price from gas price monitor Apr 8, 2021
@kyriediculous kyriediculous requested a review from yondonfu April 8, 2021 15:33
@kyriediculous
Copy link
Contributor Author

I rebased already so I could drop the gaspricemonitor commit and the diff is really tiny

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM

@kyriediculous kyriediculous merged commit f9ddb61 into master Apr 8, 2021
@kyriediculous kyriediculous deleted the nv/zero-gasprice branch April 8, 2021 18:27
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.

2 participants