Skip to content
Merged
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
8 changes: 7 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ linters:
- golint
- misspell
- govet
- ineffassign

disable:
- deadcode
- errcheck
- gosimple
- ineffassign
- staticcheck
- structcheck
- unused
Expand All @@ -26,6 +26,12 @@ issues:
# don't use default exclude rules listed in `golangci-lint run --help`
exclude-use-default: false

# Maximum issues count per one linter. Set to 0 to disable. Default is 50.
max-issues-per-linter: 0

# Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
max-same-issues: 0

exclude-rules:
# ignore govet false positive fixed in https://github.com/golang/go/issues/45043
- linters:
Expand Down
4 changes: 2 additions & 2 deletions agreement/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,13 @@ func verifyNewSeed(p unauthenticatedProposal, ledger LedgerReader) error {

if value.OriginalPeriod == 0 {
verifier := proposerRecord.SelectionID
ok, vrfOut := verifier.Verify(p.SeedProof, prevSeed)
ok, _ := verifier.Verify(p.SeedProof, prevSeed) // ignoring VrfOutput returned by Verify
if !ok {
return fmt.Errorf("payload seed proof malformed (%v, %v)", prevSeed, p.SeedProof)
}
// TODO remove the following Hash() call,
// redundant with the Verify() call above.
vrfOut, ok = p.SeedProof.Hash()
vrfOut, ok := p.SeedProof.Hash()
if !ok {
// If proof2hash fails on a proof we produced with VRF Prove, this indicates our VRF code has a dangerous bug.
// Panicking is the only safe thing to do.
Expand Down
3 changes: 2 additions & 1 deletion catchup/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ func (s *Service) fetchAndWrite(r basics.Round, prevFetchCompleteChan chan bool,
}

if s.cfg.CatchupVerifyTransactionSignatures() || s.cfg.CatchupVerifyApplyData() {
vb, err := s.ledger.Validate(s.ctx, *block, s.blockValidationPool)
var vb *ledger.ValidatedBlock
vb, err = s.ledger.Validate(s.ctx, *block, s.blockValidationPool)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tsachiherman this is the most significant issue I found — before this change, the value of err = s.ledger.AddValidatedBlock(*vb, *cert) on line 319/320 is ineffectual, and has no impact later on line 324/325 where it checks if err != nil

if err != nil {
if s.ctx.Err() != nil {
// if the context expired, just exit.
Expand Down
4 changes: 1 addition & 3 deletions cmd/goal/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
"github.com/algorand/go-algorand/config"
"github.com/algorand/go-algorand/crypto"
"github.com/algorand/go-algorand/crypto/passphrase"
"github.com/algorand/go-algorand/daemon/algod/api/spec/v1"
v1 "github.com/algorand/go-algorand/daemon/algod/api/spec/v1"
algodAcct "github.com/algorand/go-algorand/data/account"
"github.com/algorand/go-algorand/data/basics"
"github.com/algorand/go-algorand/data/transactions"
Expand Down Expand Up @@ -1195,7 +1195,6 @@ var importRootKeysCmd = &cobra.Command{
handle, err = db.MakeErasableAccessor(filepath.Join(keyDir, filename))
if err != nil {
// Couldn't open it, skip it
err = nil
continue
}

Expand All @@ -1204,7 +1203,6 @@ var importRootKeysCmd = &cobra.Command{
handle.Close()
if err != nil {
// Couldn't read it, skip it
err = nil
continue
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/tealdbg/cdtState.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ func prepareTxn(txn *transactions.Transaction, groupIndex int) []fieldDesc {
continue
}
var value string
var valType string = "string"
var valType string
tv, err := logic.TxnFieldToTealValue(txn, groupIndex, logic.TxnField(field), 0)
if err != nil {
value = err.Error()
Expand Down
18 changes: 12 additions & 6 deletions cmd/tealdbg/localLedger.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,13 @@ func getAppCreatorFromIndexer(indexerURL string, indexerToken string, app basics
queryString := fmt.Sprintf("%s/v2/applications/%d", indexerURL, app)
client := &http.Client{}
request, err := http.NewRequest("GET", queryString, nil)
if err != nil {
return basics.Address{}, fmt.Errorf("application request error: %w", err)
}
request.Header.Set("X-Indexer-API-Token", indexerToken)
resp, err := client.Do(request)
if err != nil {
return basics.Address{}, fmt.Errorf("application request error: %s", err)
return basics.Address{}, fmt.Errorf("application request error: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != 200 {
Expand All @@ -200,13 +203,13 @@ func getAppCreatorFromIndexer(indexerURL string, indexerToken string, app basics
var appResp ApplicationIndexerResponse
err = json.NewDecoder(resp.Body).Decode(&appResp)
if err != nil {
return basics.Address{}, fmt.Errorf("application response decode error: %s", err)
return basics.Address{}, fmt.Errorf("application response decode error: %w", err)
}

creator, err := basics.UnmarshalChecksumAddress(appResp.Application.Params.Creator)

if err != nil {
return basics.Address{}, fmt.Errorf("UnmarshalChecksumAddress error: %s", err)
return basics.Address{}, fmt.Errorf("UnmarshalChecksumAddress error: %w", err)
}
return creator, nil
}
Expand All @@ -215,10 +218,13 @@ func getBalanceFromIndexer(indexerURL string, indexerToken string, account basic
queryString := fmt.Sprintf("%s/v2/accounts/%s?round=%d", indexerURL, account, round)
client := &http.Client{}
request, err := http.NewRequest("GET", queryString, nil)
if err != nil {
return basics.AccountData{}, fmt.Errorf("account request error: %w", err)
}
request.Header.Set("X-Indexer-API-Token", indexerToken)
resp, err := client.Do(request)
if err != nil {
return basics.AccountData{}, fmt.Errorf("account request error: %s", err)
return basics.AccountData{}, fmt.Errorf("account request error: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != 200 {
Expand All @@ -228,11 +234,11 @@ func getBalanceFromIndexer(indexerURL string, indexerToken string, account basic
var accountResp AccountIndexerResponse
err = json.NewDecoder(resp.Body).Decode(&accountResp)
if err != nil {
return basics.AccountData{}, fmt.Errorf("account response decode error: %s", err)
return basics.AccountData{}, fmt.Errorf("account response decode error: %w", err)
}
balance, err := v2.AccountToAccountData(&accountResp.Account)
if err != nil {
return basics.AccountData{}, fmt.Errorf("AccountToAccountData error: %s", err)
return basics.AccountData{}, fmt.Errorf("AccountToAccountData error: %w", err)
}
return balance, nil
}
Expand Down
3 changes: 3 additions & 0 deletions debug/doberman/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ func main() {

// write logo
tf, err := ioutil.TempFile("", "algorand-logo.png")
if err != nil {
panic(err)
}
tfname = tf.Name()
defer func() {
time.Sleep(retireIn)
Expand Down
1 change: 0 additions & 1 deletion ledger/acctupdates.go
Original file line number Diff line number Diff line change
Expand Up @@ -1602,7 +1602,6 @@ func (au *accountUpdates) deleteStoredCatchpoints(ctx context.Context, dbQueries
err = os.Remove(absCatchpointFileName)
if err == nil || os.IsNotExist(err) {
// it's ok if the file doesn't exist. just remove it from the database and we'll be good to go.
err = nil
} else {
// we can't delete the file, abort -
return fmt.Errorf("unable to delete old catchpoint file '%s' : %v", absCatchpointFileName, err)
Expand Down
4 changes: 4 additions & 0 deletions logging/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@ func (l logger) source() *logrus.Entry {
if !ok {
file = "<???>"
line = 1
event = event.WithFields(logrus.Fields{
"file": file,
"line": line,
})
} else {
// Add file name and number
slash := strings.LastIndex(file, "/")
Expand Down
3 changes: 1 addition & 2 deletions logging/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"strings"
"time"

"github.com/satori/go.uuid"
uuid "github.com/satori/go.uuid"
"github.com/sirupsen/logrus"

"github.com/algorand/go-algorand/config"
Expand Down Expand Up @@ -186,7 +186,6 @@ func EnsureTelemetryConfigCreated(dataDir *string, genesisID string) (TelemetryC
}
created := false
if err != nil {
err = nil
created = true
cfg = createTelemetryConfig()

Expand Down
2 changes: 0 additions & 2 deletions rpcs/blockService.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ func (bs *BlockService) ServeHTTP(response http.ResponseWriter, request *http.Re
response.WriteHeader(http.StatusBadRequest)
return
}
} else {
versionStr = "1"
}
}
round, err := strconv.ParseUint(roundStr, 36, 64)
Expand Down
2 changes: 0 additions & 2 deletions rpcs/ledgerService.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ func (ls *LedgerService) ServeHTTP(response http.ResponseWriter, request *http.R
response.Write([]byte(fmt.Sprintf("invalid number of version specified %d", len(versionStrs))))
return
}
} else {
versionStr = "1"
}
}
round, err := strconv.ParseUint(roundStr, 36, 64)
Expand Down
1 change: 0 additions & 1 deletion shared/pingpong/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ func ensureAccounts(ac libgoal.Client, initCfg PpConfig) (accounts map[string]ui
}

if richestBalance >= cfg.MinAccountFunds {
srcAcctPresent = true
cfg.SrcAccount = richestAccount

fmt.Printf("Identified richest account to use for Source Account: %s -> %v\n", richestAccount, richestBalance)
Expand Down
5 changes: 1 addition & 4 deletions shared/pingpong/pingpong.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,7 @@ func computeAccountMinBalance(client libgoal.Client, cfg PpConfig) (requiredBala
fmt.Printf("required min balance for app accounts: %d\n", requiredBalance)
return
}
var fee uint64 = 1000
if cfg.MinFee > fee {
fee = cfg.MinFee
}
var fee uint64
Copy link
Contributor

Choose a reason for hiding this comment

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

while this code is technically correct, I think that we shouldn't do that as is.
Instead, we need to evaluate the bigger picture and (Maybe) eliminate the MinFee from the config file, as it seems redundant.

Copy link
Contributor Author

@cce cce Jul 16, 2021

Choose a reason for hiding this comment

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

Yes I left a comment here https://github.com/algorand/go-algorand/pull/1715/files#r671536585 about whether this was intentional or not ... I wasn't sure if this uncovered a bug or need to fix/clarify something like you're saying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pinging @algorandskiy for his opinion since these lines here were made ineffectual by #1715

Copy link
Contributor

Choose a reason for hiding this comment

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

MinFee is used in fee() func only for fee randomization. I think it is OK to remove it completely as a separate PR. For this one removing the branch is perfectly fine.

A note about using SuggestedFee here: pingpong funds accounts between iterations to ensure they have enough algos to interact, and on a high load with app/assets they some of them might not have enough algos to submit new transactions because the pool conjunction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh OK, I see so MinFee is still used in pingpong.go:WorkerState.fee(). I just wanted to make sure you realized these lines 156-159 being removed above were ineffectual (1000 is unused, and fee = cfg.MinFee is always overwritten).

if cfg.MaxFee != 0 {
fee = cfg.MaxFee
} else {
Expand Down
8 changes: 4 additions & 4 deletions test/e2e-go/cli/tealdbg/cdtmock/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ func main() {
os.Exit(1)
}

var counter int64 = 1
req := cdt.ChromeRequest{ID: counter, Method: "Debugger.Enable"}
var counter int64
counter++
req := cdt.ChromeRequest{ID: counter, Method: "Debugger.Enable"}

if err = client.SendJSON(req); err != nil {
fmt.Printf("Send error: %v", err)
Expand All @@ -101,8 +101,8 @@ func main() {
}
fmt.Printf("%s\n", string(data))

req = cdt.ChromeRequest{ID: counter, Method: "Runtime.runIfWaitingForDebugger"}
counter++
req = cdt.ChromeRequest{ID: counter, Method: "Runtime.runIfWaitingForDebugger"}

if err = client.SendJSON(req); err != nil {
fmt.Printf("Send error: %v", err)
Expand All @@ -114,8 +114,8 @@ func main() {
}
fmt.Printf("%s\n", string(data))

req = cdt.ChromeRequest{ID: counter, Method: "Debugger.resume"}
counter++
req = cdt.ChromeRequest{ID: counter, Method: "Debugger.resume"}

if err = client.SendJSON(req); err != nil {
fmt.Printf("Send error: %v", err)
Expand Down
6 changes: 2 additions & 4 deletions test/framework/fixtures/libgoalFixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,13 @@ func (f *LibGoalFixture) importRootKeys(lg *libgoal.Client, dataDir string) {
handle, err = db.MakeAccessor(filepath.Join(keyDir, filename), false, false)
if err != nil {
// Couldn't open it, skip it
err = nil
continue
}

// Fetch an account.Root from the database
root, err := account.RestoreRoot(handle)
if err != nil {
// Couldn't read it, skip it
err = nil
continue
}

Expand All @@ -177,15 +175,13 @@ func (f *LibGoalFixture) importRootKeys(lg *libgoal.Client, dataDir string) {
handle, err = db.MakeErasableAccessor(filepath.Join(keyDir, filename))
if err != nil {
// Couldn't open it, skip it
err = nil
continue
}

// Fetch an account.Participation from the database
participation, err := account.RestoreParticipation(handle)
if err != nil {
// Couldn't read it, skip it
err = nil
handle.Close()
continue
}
Expand Down Expand Up @@ -224,6 +220,7 @@ func (f *LibGoalFixture) GetLibGoalClientFromDataDir(dataDir string) libgoal.Cli
// GetLibGoalClientForNamedNode returns the LibGoal Client for a given named node
func (f *LibGoalFixture) GetLibGoalClientForNamedNode(nodeName string) libgoal.Client {
nodeDir, err := f.network.GetNodeDir(nodeName)
f.failOnError(err, "network.GetNodeDir failed: %v")
client, err := libgoal.MakeClientWithBinDir(f.binDir, nodeDir, nodeDir, libgoal.KmdClient)
f.failOnError(err, "make libgoal client failed: %v")
f.importRootKeys(&client, nodeDir)
Expand All @@ -245,6 +242,7 @@ func (f *LibGoalFixture) GetLibGoalClientFromDataDirNoKeys(dataDir string) libgo
// GetLibGoalClientForNamedNodeNoKeys returns the LibGoal Client for a given named node
func (f *LibGoalFixture) GetLibGoalClientForNamedNodeNoKeys(nodeName string) libgoal.Client {
nodeDir, err := f.network.GetNodeDir(nodeName)
f.failOnError(err, "network.GetNodeDir failed: %v")
client, err := libgoal.MakeClientWithBinDir(f.binDir, nodeDir, nodeDir, libgoal.AlgodClient)
f.failOnError(err, "make libgoal client failed: %v")
return client
Expand Down