Skip to content
This repository has been archived by the owner on Nov 24, 2022. It is now read-only.

add client filtering flag #75

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .bingo/Variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ $(BUF): $(BINGO_DIR)/buf.mod
@echo "(re)installing $(GOBIN)/buf-v0.41.0"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=buf.mod -o=$(GOBIN)/buf-v0.41.0 "github.com/bufbuild/buf/cmd/buf"

GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.39.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just updating golangci-lint.

GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.43.0
$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
@echo "(re)installing $(GOBIN)/golangci-lint-v1.39.0"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.39.0 "github.com/golangci/golangci-lint/cmd/golangci-lint"
@echo "(re)installing $(GOBIN)/golangci-lint-v1.43.0"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.43.0 "github.com/golangci/golangci-lint/cmd/golangci-lint"

GOMPLATE := $(GOBIN)/gomplate-v3.9.0
$(GOMPLATE): $(BINGO_DIR)/gomplate.mod
Expand Down
2 changes: 1 addition & 1 deletion .bingo/golangci-lint.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT

go 1.16

require github.com/golangci/golangci-lint v1.39.0 // cmd/golangci-lint
require github.com/golangci/golangci-lint v1.43.0 // cmd/golangci-lint
2 changes: 1 addition & 1 deletion .bingo/variables.env
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ BINGO="${GOBIN}/bingo-v0.4.0"

BUF="${GOBIN}/buf-v0.41.0"

GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.39.0"
GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.43.0"

GOMPLATE="${GOBIN}/gomplate-v3.9.0"

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,11 @@ Problems can arise, especially when you are new to bidbot. Here are some common
- Try several times with the Glif node that Bidbot uses - `FULLNODE_API_INFO="https://api.node.glif.io" lotus net findpeer YOUR_MINER_PEER_ID`.
* Or you set too small a `--deal-start-window`. You would see something like `deal rejected: cannot seal a sector before ...` in your miner log. Set your `deal-start-window` to be a few hours longer than `ExpectedSealDuration` in your miner config.

1. Too many deals flood in, exceeding you sealing capacity. Two ways to throttle.
2. Too many deals flood in, exceeding you sealing capacity. Two ways to throttle.
* `--running-bytes-limit` to limit the total bytes bidbot processes in a period of time.
* `--sealing-sectors-limit` to pause bidding if the miner node has more than desired sectors in sealing.

1. We've seen a case when bidbot importing data long after winning the bids, it got errors like "normal shutdown of state machine" or "given data does not match expected commP ..." even though the real problem was the deal start epoch being exceeded. When in doubt, check your `lotus-miner` logs around that time for the real reason.
3. We've seen a case when bidbot importing data long after winning the bids, it got errors like "normal shutdown of state machine" or "given data does not match expected commP ..." even though the real problem was the deal start epoch being exceeded. When in doubt, check your `lotus-miner` logs around that time for the real reason.

It is also suggested to set `--log-debug=true` and keep the logs when you want to seek help from Textile or the community.

Expand Down
7 changes: 6 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ Zero means no limits`,
DefValue: "3h",
Description: `The timeout to fetch deal data. Be conservative to leave enough room for network instability.`,
},

{
Name: "client-address-whitelist",
DefValue: "",
Description: `If not empty, only bid in auctions from the comma-separated list of client addresses`,
},
{
Name: "cid-gravity-key",
DefValue: "",
Expand Down Expand Up @@ -407,6 +411,7 @@ var daemonCmd = &cobra.Command{
Min: v.GetUint64("deal-size-min"),
Max: v.GetUint64("deal-size-max"),
},
ClientAddressWhitelist: strings.Split(v.GetString("client-address-whitelist"), ","),
},
BytesLimiter: bytesLimiter,
ConcurrentImports: v.GetInt("concurrent-imports-limit"),
Expand Down
21 changes: 21 additions & 0 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sync/atomic"
"time"

"github.com/filecoin-project/go-address"
"github.com/ipfs/go-cid"
"github.com/libp2p/go-libp2p-core/crypto"
core "github.com/libp2p/go-libp2p-core/peer"
Expand Down Expand Up @@ -121,6 +122,8 @@ type AuctionFilters struct {
DealDuration MinMaxFilter
// DealSize sets the min and max deal size to bid on.
DealSize MinMaxFilter
// ClientAddressWhitelist if not empty only allows bidding in auctions from a clients list.
ClientAddressWhitelist []string
}

// Validate ensures AuctionFilters are valid.
Expand All @@ -131,6 +134,11 @@ func (f *AuctionFilters) Validate() error {
if err := f.DealDuration.Validate(); err != nil {
return fmt.Errorf("invalid deal size filter: %v", err)
}
for _, clientAddress := range f.ClientAddressWhitelist {
if _, err := address.NewFromString(clientAddress); err != nil {
return fmt.Errorf("invalid client address filter %s: %s", clientAddress, err)
}
}
return nil
}

Expand Down Expand Up @@ -443,6 +451,19 @@ func (s *Service) filterAuction(auction *pb.Auction) (rejectReason string) {
s.auctionFilters.DealDuration.Max)
}

if len(s.auctionFilters.ClientAddressWhitelist) > 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gist of PR.

var isWhitelistedClient bool
for _, clientAddress := range s.auctionFilters.ClientAddressWhitelist {
if clientAddress == auction.ClientAddress {
isWhitelistedClient = true
break
}
}
if !isWhitelistedClient {
return fmt.Sprintf("auction is from client %s which isn't whitelisted", auction.ClientAddress)
}
}

return ""
}

Expand Down
49 changes: 49 additions & 0 deletions service/service_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package service

import (
"testing"
"time"

"github.com/stretchr/testify/require"
pb "github.com/textileio/bidbot/gen/v1"
core "github.com/textileio/bidbot/lib/auction"
"google.golang.org/protobuf/types/known/timestamppb"
)

func TestClientFilter(t *testing.T) {
t.Parallel()

// Start with no client filters
s := Service{
auctionFilters: AuctionFilters{
DealDuration: MinMaxFilter{
Min: core.MinDealDuration,
Max: core.MaxDealDuration,
},
DealSize: MinMaxFilter{
Min: 1,
Max: 10,
},
},
}
auction := &pb.Auction{
DealSize: 5,
DealDuration: core.MinDealDuration,
EndsAt: timestamppb.New(time.Now().Add(time.Minute)),
ClientAddress: "f2kb4izxsxu2jyyslzwmv2sfbrgpld56efedgru5i",
}

t.Run("accept-any-client-address", func(t *testing.T) {
require.Empty(t, s.filterAuction(auction))
})
Comment on lines +36 to +38
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test that if no whitelist is provided, we're accepting any auction.


s.auctionFilters.ClientAddressWhitelist = []string{"f144zep4gitj73rrujd3jw6iprljicx6vl4wbeavi"}
t.Run("reject-single-client-address", func(t *testing.T) {
require.Contains(t, s.filterAuction(auction), "f2kb4izxsxu2jyyslzwmv2sfbrgpld56efedgru5i")
})
Comment on lines +40 to +43
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test with a whitelist only containing f144zep4gitj73rrujd3jw6iprljicx6vl4wbeavi.

Then test if an auction from client f2kb4izxsxu2jyyslzwmv2sfbrgpld56efedgru5i (L33) will be rejected, since that must be the case.

The rejection message should be a string explaining the cause of rejection, which must say something about f2kb4izxsxu2jyyslzwmv2sfbrgpld56efedgru5i being rejected from not being in the whitelist. The logic in the code never have a strict interpretation of the returned string, its only using for logging; so took the liberty of avoiding type-erroring.


auction.ClientAddress = s.auctionFilters.ClientAddressWhitelist[0]
t.Run("accept-single-client-address", func(t *testing.T) {
require.Empty(t, s.filterAuction(auction))
})
Comment on lines +45 to +48
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test the case where the auction client address matches a value in the whitelist, and check that the auction isn't rejected.

}
16 changes: 16 additions & 0 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,22 @@ func TestNew(t *testing.T) {
})
require.Error(t, err)

// Incorrect string representation of whitelisted client address.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Augment an existing test to check fast-fail in parsing a malformed client address in a whitelist.

_, err = newService(t, func(config *service.Config) {
config.AuctionFilters = service.AuctionFilters{
DealDuration: service.MinMaxFilter{
Min: 10,
Max: 20,
},
DealSize: service.MinMaxFilter{
Min: 10,
Max: 20,
},
ClientAddressWhitelist: []string{"invalidClientAddr"},
}
})
require.Error(t, err)

// Good config
s, err := newService(t, nil)
require.NoError(t, err)
Expand Down