Skip to content

Commit

Permalink
Merge pull request #1838 from livepeer/nv/remove-flags
Browse files Browse the repository at this point in the history
cmd,eth: remove deprecated flags
  • Loading branch information
kyriediculous authored Apr 13, 2021
2 parents 218e13f + 300d589 commit fe6a6de
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 74 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@

### Features ⚒

#### General

- \#1838 Remove deprecated flags: -gasPrice, -s3bucket, -s3creds, -gsbucket, -gskey (@kyriediculous)

#### Broadcaster

- \#1823 Mark more transcoder errors as NonRetryable (@jailuthra)
2 changes: 1 addition & 1 deletion cmd/devtool/devtool.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func ethSetup(ethAcctAddr, keystoreDir string, isBroadcaster bool) {
return
}

if err := client.SetGasInfo(0, nil); err != nil {
if err := client.SetGasInfo(0); err != nil {
glog.Errorf("Failed to set gas info on Livepeer Ethereum Client: %v", err)
return
}
Expand Down
52 changes: 2 additions & 50 deletions cmd/livepeer/livepeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ func main() {
ethOrchAddr := flag.String("ethOrchAddr", "", "ETH address of an on-chain registered orchestrator")
ethUrl := flag.String("ethUrl", "", "Ethereum node JSON-RPC URL")
gasLimit := flag.Int("gasLimit", 0, "Gas limit for ETH transactions")
gasPrice := flag.Int("gasPrice", 0, "Gas price for ETH transactions")
maxGasPrice := flag.Int("maxGasPrice", 0, "Maximum gas price for ETH transactions")
ethController := flag.String("ethController", "", "Protocol smart contract address")
initializeRound := flag.Bool("initializeRound", false, "Set to true if running as a transcoder and the node should automatically initialize new rounds")
Expand Down Expand Up @@ -166,12 +165,6 @@ func main() {
objectstore := flag.String("objectStore", "", "url of primary object store")
recordstore := flag.String("recordStore", "", "url of object store for recodings")

// All deprecated
s3bucket := flag.String("s3bucket", "", "S3 region/bucket (e.g. eu-central-1/testbucket)")
s3creds := flag.String("s3creds", "", "S3 credentials (in form ACCESSKEYID/ACCESSKEY)")
gsBucket := flag.String("gsbucket", "", "Google storage bucket")
gsKey := flag.String("gskey", "", "Google Storage private key file name (in json format)")

// API
authWebhookURL := flag.String("authWebhookUrl", "", "RTMP authentication webhook URL")
orchWebhookURL := flag.String("orchWebhookUrl", "", "Orchestrator discovery callback URL")
Expand Down Expand Up @@ -390,13 +383,7 @@ func main() {
return
}

var bigGasPrice *big.Int
if *gasPrice > 0 {
glog.Warning("-gasPrice is deprecated, you can use -maxGasPrice instead")
bigGasPrice = big.NewInt(int64(*gasPrice))
}

if err := client.SetGasInfo(uint64(*gasLimit), bigGasPrice); err != nil {
if err := client.SetGasInfo(uint64(*gasLimit)); err != nil {
glog.Errorf("Failed to set gas info on Livepeer Ethereum Client: %v", err)
return
}
Expand Down Expand Up @@ -724,41 +711,6 @@ func main() {
}()
}

if *s3bucket != "" && *s3creds == "" || *s3bucket == "" && *s3creds != "" {
glog.Error("Should specify both s3bucket and s3creds")
return
}
if *gsBucket != "" && *gsKey == "" || *gsBucket == "" && *gsKey != "" {
glog.Error("Should specify both gsbucket and gskey")
return
}

// XXX get s3 credentials from local env vars?
if *s3bucket != "" && *s3creds != "" {
br := strings.Split(*s3bucket, "/")
cr := strings.Split(*s3creds, "/")
u := url.URL{
Scheme: "s3",
Host: br[0],
Path: fmt.Sprintf("/%s", br[1]),
User: url.UserPassword(cr[0], cr[1]),
}
glog.Warningf("-s3bucket and -s3creds are deprecated. Instead, you can use -objectStore %s", u.String())
ustr := u.String()
objectstore = &ustr
}

if *gsBucket != "" && *gsKey != "" {
u := url.URL{
Scheme: "gs",
Host: *gsBucket,
RawQuery: fmt.Sprintf("keyfile=%s", *gsKey),
}
glog.Warningf("-gsbucket and -gskey are deprecated. Instead, you can use -objectStore %s", u.String())
ustr := u.String()
objectstore = &ustr
}

if *objectstore != "" {
prepared, err := drivers.PrepareOSURL(*objectstore)
if err != nil {
Expand Down Expand Up @@ -865,7 +817,7 @@ func main() {
// Set the verifier path. Remove once [1] is implemented!
// [1] https://github.com/livepeer/verification-classifier/issues/64
if drivers.NodeStorage == nil && *verifierPath == "" {
glog.Fatal("Requires a path to the verifier shared volume when local storage is in use; use -verifierPath, S3 or GCS")
glog.Fatal("Requires a path to the verifier shared volume when local storage is in use; use -verifierPath or -objectStore")
}
verification.VerifierPath = *verifierPath
} else if *localVerify {
Expand Down
6 changes: 2 additions & 4 deletions eth/accountmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package eth
import (
"errors"
"fmt"
"math/big"

"github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/accounts/abi/bind"
Expand All @@ -24,7 +23,7 @@ var (
type AccountManager interface {
Unlock(passphrase string) error
Lock() error
CreateTransactOpts(gasLimit uint64, gasPrice *big.Int) (*bind.TransactOpts, error)
CreateTransactOpts(gasLimit uint64) (*bind.TransactOpts, error)
SignTx(tx *types.Transaction) (*types.Transaction, error)
Sign(msg []byte) ([]byte, error)
Account() accounts.Account
Expand Down Expand Up @@ -120,15 +119,14 @@ func (am *accountManager) Lock() error {

// Create transact opts for client use - account must be unlocked
// Can optionally set gas limit and gas price used
func (am *accountManager) CreateTransactOpts(gasLimit uint64, gasPrice *big.Int) (*bind.TransactOpts, error) {
func (am *accountManager) CreateTransactOpts(gasLimit uint64) (*bind.TransactOpts, error) {
if !am.unlocked {
return nil, ErrLocked
}

return &bind.TransactOpts{
From: am.account.Address,
GasLimit: gasLimit,
GasPrice: gasPrice,
Signer: func(signer types.Signer, address ethcommon.Address, tx *types.Transaction) (*types.Transaction, error) {
if address != am.account.Address {
return nil, errors.New("not authorized to sign this account")
Expand Down
12 changes: 3 additions & 9 deletions eth/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ type LivepeerEthClient interface {
CheckTx(*types.Transaction) error
ReplaceTransaction(*types.Transaction, string, *big.Int) (*types.Transaction, error)
Sign([]byte) ([]byte, error)
GetGasInfo() (uint64, *big.Int)
SetGasInfo(uint64, *big.Int) error
SetGasInfo(uint64) error
}

type client struct {
Expand Down Expand Up @@ -344,8 +343,8 @@ func (c *client) setContracts(opts *bind.TransactOpts) error {
return nil
}

func (c *client) SetGasInfo(gasLimit uint64, gasPrice *big.Int) error {
opts, err := c.accountManager.CreateTransactOpts(gasLimit, gasPrice)
func (c *client) SetGasInfo(gasLimit uint64) error {
opts, err := c.accountManager.CreateTransactOpts(gasLimit)
if err != nil {
return err
}
Expand All @@ -354,15 +353,10 @@ func (c *client) SetGasInfo(gasLimit uint64, gasPrice *big.Int) error {
return err
} else {
c.gasLimit = gasLimit
c.gasPrice = gasPrice
return nil
}
}

func (c *client) GetGasInfo() (gasLimit uint64, gasPrice *big.Int) {
return c.gasLimit, c.gasPrice
}

func (c *client) Account() accounts.Account {
return c.accountManager.Account()
}
Expand Down
5 changes: 2 additions & 3 deletions eth/stubclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,8 @@ func (c *StubClient) CheckTx(tx *types.Transaction) error {
func (c *StubClient) ReplaceTransaction(tx *types.Transaction, method string, gasPrice *big.Int) (*types.Transaction, error) {
return nil, nil
}
func (c *StubClient) Sign(msg []byte) ([]byte, error) { return msg, c.Err }
func (c *StubClient) GetGasInfo() (uint64, *big.Int) { return 0, nil }
func (c *StubClient) SetGasInfo(uint64, *big.Int) error { return nil }
func (c *StubClient) Sign(msg []byte) ([]byte, error) { return msg, c.Err }
func (c *StubClient) SetGasInfo(uint64) error { return nil }

// Faucet
func (c *StubClient) NextValidRequest(common.Address) (*big.Int, error) { return nil, nil }
Expand Down
17 changes: 10 additions & 7 deletions test_args.sh
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,18 @@ else
[ ! -d "$CUSTOM_DATADIR"/rinkeby ] # sanity check that network isn't included
kill $pid

# Check that price can be set to 0 with -pricePerUnit 0
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -s3bucket abc -pricePerUnit 0 -network rinkeby $ETH_ARGS 2>&1 | grep "Should specify both s3bucket and s3creds"
# Check that -pricePerUnit needs to be set
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -s3bucket abc -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pricePerUnit must be set"
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pricePerUnit must be set"
# Orchestrator needs PricePerUnit > 0
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -s3bucket abc -pricePerUnit -5 -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pricePerUnit must be >= 0, provided -5"
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -pricePerUnit -5 -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pricePerUnit must be >= 0, provided -5"
# Orchestrator needs PixelsPerUnit > 0
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -s3bucket abc -pixelsPerUnit 0 -pricePerUnit 5 -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pixelsPerUnit must be > 0, provided 0"
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -s3bucket abc -pixelsPerUnit -5 -pricePerUnit 5 -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pixelsPerUnit must be > 0, provided -5"
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -pixelsPerUnit 0 -pricePerUnit 5 -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pixelsPerUnit must be > 0, provided 0"
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -pixelsPerUnit -5 -pricePerUnit 5 -network rinkeby $ETH_ARGS 2>&1 | grep -e "-pixelsPerUnit must be > 0, provided -5"

# Check that price can be set to 0 with -pricePerUnit 0
res=0
$TMPDIR/livepeer -orchestrator -serviceAddr 127.0.0.1:8935 -transcoder -pricePerUnit 0 -network rinkeby $ETH_ARGS || res=$?
[ $res -ne 0 ]

# Broadcaster needs a valid rational number for -maxTicketEV
res=0
Expand Down Expand Up @@ -217,7 +220,7 @@ run_lp -broadcaster -verifierUrl http://host -verifierPath path
kill $pid

# Check OK with verifier + external storage
run_lp -broadcaster -verifierUrl http://host -s3bucket foo/bar -s3creds baz/bat
run_lp -broadcaster -verifierUrl http://host -objectStore s3+https://ACCESS_KEY_ID:ACCESS_KEY_PASSWORD@s3api.example.com/bucket-name
kill $pid

# Check that HTTP ingest is disabled when -httpAddr is publicly accessible and there is no auth webhook URL and -httpIngest defaults to false
Expand Down

0 comments on commit fe6a6de

Please sign in to comment.