go-algorand 4.1.0-stable Release PR

.golangci.yml

@@ -15,10 +15,10 @@ linters:
     - misspell
     - nilerr
     - nolintlint
+    - paralleltest
     - revive
     - staticcheck
     - typecheck
-    - paralleltest
     - unused
 
 severity:
@@ -124,6 +124,8 @@ issues:
       linters: unused
     - path: tools/
       linters: unused
+    - path: daemon/kmd/lib/kmdapi/
+      linters: unused
     - path: _test\.go
       linters:
         - errcheck
@@ -142,6 +144,9 @@ issues:
       linters:
         - staticcheck
       text: "SA4006: this value" # of X is never used
+    - linters:
+        - staticcheck
+      text: "(SA3001|SA1019):"
     - path: _test\.go
       linters:
         - revive

Makefile

@@ -51,11 +51,6 @@ export GOTESTCOMMAND=gotestsum --format pkgname --jsonfile testresults.json --
 endif
 
 ifeq ($(OS_TYPE), darwin)
-# For Xcode >= 15, set -no_warn_duplicate_libraries linker option
-CLANG_MAJOR_VERSION := $(shell clang --version | grep '^Apple clang version ' | awk '{print $$4}' | cut -d. -f1)
-ifeq ($(shell [ $(CLANG_MAJOR_VERSION) -ge 15 ] && echo true), true)
-EXTLDFLAGS := -Wl,-no_warn_duplicate_libraries
-endif
 # M1 Mac--homebrew install location in /opt/homebrew
 ifeq ($(ARCH), arm64)
 export CPATH=/opt/homebrew/include
@@ -148,6 +143,13 @@ generate: deps
 
 msgp: $(patsubst %,%/msgp_gen.go,$(MSGP_GENERATE))
 
+api:
+	make -C daemon/algod/api
+
+logic:
+	make -C data/transactions/logic
+
+
 %/msgp_gen.go: deps ALWAYS
 		@set +e; \
 		printf "msgp: $(@D)..."; \
@@ -335,7 +337,11 @@ node_exporter: $(GOPATH1)/bin/node_exporter
 $(GOPATH1)/bin/node_exporter:
 	mkdir -p $(GOPATH1)/bin && \
 	cd $(GOPATH1)/bin && \
-	tar -xzvf $(SRCPATH)/installer/external/node_exporter-stable-$(shell ./scripts/ostype.sh)-$(shell uname -m | tr '[:upper:]' '[:lower:]').tar.gz && \
+	if [ -z "$(CROSS_COMPILE_ARCH)" ]; then \
+		tar -xzvf $(SRCPATH)/installer/external/node_exporter-stable-$(shell ./scripts/ostype.sh)-$(shell uname -m | tr '[:upper:]' '[:lower:]').tar.gz; \
+	else \
+		tar -xzvf $(SRCPATH)/installer/external/node_exporter-stable-$(shell ./scripts/ostype.sh)-universal.tar.gz; \
+	fi && \
 	cd -
 
 # deploy

agreement/actions.go

@@ -566,7 +566,6 @@ func (c checkpointAction) do(ctx context.Context, s *Service) {
 		// we don't expect this to happen in recovery
 		s.log.with(logEvent).Errorf("checkpoint action for (%v, %v, %v) reached with nil completion channel", c.Round, c.Period, c.Step)
 	}
-	return
 }
 
 func (c checkpointAction) String() string {

agreement/agreementtest/simulate_test.go

@@ -207,7 +207,7 @@ func (l *testLedger) LookupAgreement(r basics.Round, a basics.Address) (basics.O
 		err := fmt.Errorf("Lookup called on future round: %v > %v! (this is probably a bug)", r, l.nextRound)
 		panic(err)
 	}
-	return l.state[a].OnlineAccountData(), nil
+	return basics_testing.OnlineAccountData(l.state[a]), nil
 }
 
 func (l *testLedger) Circulation(r basics.Round, voteRnd basics.Round) (basics.MicroAlgos, error) {
@@ -222,7 +222,7 @@ func (l *testLedger) Circulation(r basics.Round, voteRnd basics.Round) (basics.M
 	var sum basics.MicroAlgos
 	var overflowed bool
 	for _, rec := range l.state {
-		sum, overflowed = basics.OAddA(sum, rec.OnlineAccountData().VotingStake())
+		sum, overflowed = basics.OAddA(sum, basics_testing.OnlineAccountData(rec).VotingStake())
 		if overflowed {
 			panic("circulation computation overflowed")
 		}

agreement/autopsy.go

@@ -384,7 +384,7 @@ func (a *Autopsy) extractNextCdv(ch chan<- autopsyTrace) (bounds AutopsyBounds,
 				close(pch)
 			}
 
-			pch = make(chan autopsyPair, 0)
+			pch = make(chan autopsyPair)
 			acc = autopsyTrace{m: acc.m, p: pch}
 			err = protocol.DecodeStream(a, &acc.x)
 			if err != nil {

agreement/certificate_test.go

@@ -276,7 +276,7 @@ func TestCertificateCertWrongRound(t *testing.T) {
 	ledger, addresses, vrfSecrets, otSecrets := readOnlyFixture100()
 	round := ledger.NextRound()
 	period := period(0)
-	block := makeRandomBlock(1 - 1)
+	block := makeRandomBlock(0)
 
 	votes := make([]vote, 0)
 	equiVotes := make([]equivocationVote, 0)

agreement/common_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/algorand/go-algorand/config"
 	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/data/basics"
+	basics_testing "github.com/algorand/go-algorand/data/basics/testing"
 	"github.com/algorand/go-algorand/data/bookkeeping"
 	"github.com/algorand/go-algorand/data/committee"
 	"github.com/algorand/go-algorand/logging"
@@ -332,7 +333,7 @@ func (l *testLedger) LookupAgreement(r basics.Round, a basics.Address) (basics.O
 		return basics.OnlineAccountData{}, &LedgerDroppedRoundError{}
 	}
 
-	return l.state[a].OnlineAccountData(), nil
+	return basics_testing.OnlineAccountData(l.state[a]), nil
 }
 
 func (l *testLedger) Circulation(r basics.Round, voteRnd basics.Round) (basics.MicroAlgos, error) {
@@ -347,7 +348,7 @@ func (l *testLedger) Circulation(r basics.Round, voteRnd basics.Round) (basics.M
 	var sum basics.MicroAlgos
 	var overflowed bool
 	for _, rec := range l.state {
-		sum, overflowed = basics.OAddA(sum, rec.OnlineAccountData().VotingStake())
+		sum, overflowed = basics.OAddA(sum, basics_testing.OnlineAccountData(rec).VotingStake())
 		if overflowed {
 			panic("circulation computation overflowed")
 		}

agreement/fuzzer/ledger_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/algorand/go-algorand/config"
 	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/data/basics"
+	basics_testing "github.com/algorand/go-algorand/data/basics/testing"
 	"github.com/algorand/go-algorand/data/bookkeeping"
 	"github.com/algorand/go-algorand/data/committee"
 	"github.com/algorand/go-algorand/protocol"
@@ -241,7 +242,7 @@ func (l *testLedger) LookupAgreement(r basics.Round, a basics.Address) (basics.O
 		err := fmt.Errorf("Lookup called on future round: %d >= %d! (this is probably a bug)", r, l.nextRound)
 		panic(err)
 	}
-	return l.state[a].OnlineAccountData(), nil
+	return basics_testing.OnlineAccountData(l.state[a]), nil
 }
 
 func (l *testLedger) Circulation(r basics.Round, voteRnd basics.Round) (basics.MicroAlgos, error) {
@@ -256,7 +257,7 @@ func (l *testLedger) Circulation(r basics.Round, voteRnd basics.Round) (basics.M
 	var sum basics.MicroAlgos
 	var overflowed bool
 	for _, rec := range l.state {
-		sum, overflowed = basics.OAddA(sum, rec.OnlineAccountData().VotingStake())
+		sum, overflowed = basics.OAddA(sum, basics_testing.OnlineAccountData(rec).VotingStake())
 		if overflowed {
 			panic("circulation computation overflowed")
 		}

agreement/gossip/network.go

@@ -20,7 +20,6 @@ package gossip
 
 import (
 	"context"
-	"time"
 
 	"github.com/algorand/go-algorand/agreement"
 	"github.com/algorand/go-algorand/config"
@@ -179,12 +178,3 @@ func (i *networkImpl) Disconnect(h agreement.MessageHandle) {
 
 	i.net.Disconnect(metadata.raw.Sender)
 }
-
-// broadcastTimeout is currently only used by test code.
-// In test code we want to queue up a bunch of outbound packets and then see that they got through, so we need to wait at least a little bit for them to all go out.
-// Normal agreement state machine code uses GossipNode.Broadcast non-blocking and may drop outbound packets.
-func (i *networkImpl) broadcastTimeout(t protocol.Tag, data []byte, timeout time.Duration) error {
-	ctx, cancel := context.WithTimeout(context.Background(), timeout)
-	defer cancel()
-	return i.net.Broadcast(ctx, t, data, true, nil)
-}

agreement/persistence.go

@@ -355,7 +355,7 @@ func (p *asyncPersistenceLoop) loop(ctx context.Context) {
 		select {
 		case <-ctx.Done():
 			return
-		case s, _ = <-p.pending:
+		case s = <-p.pending:
 		}
 
 		// make sure that the ledger finished writing the previous round to disk.

agreement/player.go

@@ -275,7 +275,7 @@ func (p *player) issueFastVote(r routerHandle) (actions []action) {
 
 func (p *player) handleCheckpointEvent(r routerHandle, e checkpointEvent) []action {
 	return []action{
-		checkpointAction{
+		checkpointAction{ //nolint:gosimple // explicit assignment for clarity
 			Round:  e.Round,
 			Period: e.Period,
 			Step:   e.Step,

agreement/vote.go

@@ -146,6 +146,12 @@ func (uv unauthenticatedVote) verify(l LedgerReader) (vote, error) {
 	return vote{R: rv, Cred: cred, Sig: uv.Sig}, nil
 }
 
+var (
+	// testMakeVoteCheck is a function that can be set to check every
+	// unauthenticatedVote before it is returned by makeVote. It is only set by tests.
+	testMakeVoteCheck func(*unauthenticatedVote) error
+)
+
 // makeVote creates a new unauthenticated vote from its constituent components.
 //
 // makeVote returns an error if it fails.
@@ -178,7 +184,15 @@ func makeVote(rv rawVote, voting crypto.OneTimeSigner, selection *crypto.VRFSecr
 	}
 
 	cred := committee.MakeCredential(&selection.SK, m.Selector)
-	return unauthenticatedVote{R: rv, Cred: cred, Sig: sig}, nil
+	ret := unauthenticatedVote{R: rv, Cred: cred, Sig: sig}
+
+	// for use when running in tests
+	if testMakeVoteCheck != nil {
+		if testErr := testMakeVoteCheck(&ret); testErr != nil {
+			return unauthenticatedVote{}, fmt.Errorf("makeVote: testMakeVoteCheck failed: %w", testErr)
+		}
+	}
+	return ret, nil
 }
 
 // ToBeHashed implements the Hashable interface.

agreement/vote_test.go

@@ -17,6 +17,10 @@
 package agreement
 
 import (
+	"bytes"
+	"encoding/base64"
+	"encoding/hex"
+	"fmt"
 	"os"
 	"testing"
 
@@ -27,10 +31,38 @@ import (
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/data/committee"
 	"github.com/algorand/go-algorand/logging"
+	"github.com/algorand/go-algorand/network/vpack"
 	"github.com/algorand/go-algorand/protocol"
 	"github.com/algorand/go-algorand/test/partitiontest"
 )
 
+func init() {
+	testMakeVoteCheck = testVPackMakeVote
+}
+
+func testVPackMakeVote(v *unauthenticatedVote) error {
+	vbuf := protocol.Encode(v)
+	enc := vpack.NewStatelessEncoder()
+	dec := vpack.NewStatelessDecoder()
+	encBuf, err := enc.CompressVote(nil, vbuf)
+	if err != nil {
+		return fmt.Errorf("makeVote: failed to parse vote msgpack: %v", err)
+	}
+	decBuf, err := dec.DecompressVote(nil, encBuf)
+	if err != nil {
+		return fmt.Errorf("makeVote: failed to decompress vote msgpack: %v", err)
+	}
+	if !bytes.Equal(vbuf, decBuf) {
+		fmt.Printf("vote: %+v\n", v)
+		fmt.Printf("oldbuf: %s\n", hex.EncodeToString(vbuf))
+		fmt.Printf("decbuf: %s\n", hex.EncodeToString(decBuf))
+		fmt.Printf("base64 oldbuf: %s\n", base64.StdEncoding.EncodeToString(vbuf))
+		fmt.Printf("base64 decbuf: %s\n", base64.StdEncoding.EncodeToString(decBuf))
+		return fmt.Errorf("makeVote: decompressed vote msgpack does not match original")
+	}
+	return nil
+}
+
 // error is set if this address is not selected
 func makeVoteTesting(addr basics.Address, vrfSecs *crypto.VRFSecrets, otSecs crypto.OneTimeSigner, ledger Ledger, round basics.Round, period period, step step, digest crypto.Digest) (vote, error) {
 	var proposal proposalValue

buildnumber.dat

@@ -1 +1 @@
-3
+0

catchup/catchpointService.go

@@ -763,9 +763,8 @@ func (cs *CatchpointCatchupService) updateNodeCatchupMode(catchupModeEnabled boo
 	case newCtx, open := <-newCtxCh:
 		if open {
 			cs.ctx, cs.cancelCtxFunc = context.WithCancel(newCtx)
-		} else {
-			// channel is closed, this means that the node is stopping
 		}
+		// if channel is closed, this means that the node is stopping
 	case <-cs.ctx.Done():
 		// the node context was canceled before the SetCatchpointCatchupMode goroutine had
 		// the chance of completing. We At this point, the service is shutting down. However,
@@ -823,12 +822,14 @@ func (cs *CatchpointCatchupService) checkLedgerDownload() error {
 	for i := 0; i < cs.config.CatchupLedgerDownloadRetryAttempts; i++ {
 		psp, peerError := cs.blocksDownloadPeerSelector.getNextPeer()
 		if peerError != nil {
-			return err
+			cs.log.Debugf("checkLedgerDownload: error on getNextPeer: %s", peerError.Error())
+			return peerError
 		}
 		err = ledgerFetcher.headLedger(context.Background(), psp.Peer, round)
 		if err == nil {
 			return nil
 		}
+		cs.log.Debugf("checkLedgerDownload: failed to headLedger from peer %s: %v", peerAddress(psp.Peer), err)
 		// a non-nil error means that the catchpoint is not available, so we should rank it accordingly
 		cs.blocksDownloadPeerSelector.rankPeer(psp, peerRankNoCatchpointForRound)
 	}

catchup/classBasedPeerSelector.go

@@ -18,9 +18,10 @@ package catchup
 
 import (
 	"errors"
+	"time"
+
 	"github.com/algorand/go-algorand/network"
 	"github.com/algorand/go-deadlock"
-	"time"
 )
 
 // classBasedPeerSelector is a rankPooledPeerSelector that tracks and ranks classes of peers based on their response behavior.
@@ -56,8 +57,13 @@ func (c *classBasedPeerSelector) rankPeer(psp *peerSelectorPeer, rank int) (int,
 		}
 
 		// Peer was in this class, if there was any kind of download issue, we increment the failure count
-		if rank >= peerRankNoBlockForRound {
+		failure := rank >= peerRankNoBlockForRound
+		if failure {
 			wp.downloadFailures++
+		} else {
+			// class usually multiple peers and we do not want to punish the entire class for one peer's failure
+			// by decrementing the downloadFailures
+			wp.downloadFailures = max(wp.downloadFailures-1, 0)
 		}
 
 		break

catchup/classBasedPeerSelector_test.go

@@ -17,11 +17,12 @@
 package catchup
 
 import (
+	"testing"
+	"time"
+
 	"github.com/algorand/go-algorand/network"
 	"github.com/algorand/go-algorand/test/partitiontest"
 	"github.com/stretchr/testify/require"
-	"testing"
-	"time"
 )
 
 // Use to mock the wrapped peer selectors where warranted
@@ -481,6 +482,7 @@ func TestClassBasedPeerSelector_integration(t *testing.T) {
 	require.Nil(t, err)
 	require.Equal(t, mockP1WrappedPeer, peerResult)
 	cps.rankPeer(mockP1WrappedPeer, peerRankNoBlockForRound)
+	cps.rankPeer(mockP1WrappedPeer, peerRankNoBlockForRound)
 
 	peerResult, err = cps.getNextPeer()
 	require.Nil(t, err)
@@ -495,4 +497,8 @@ func TestClassBasedPeerSelector_integration(t *testing.T) {
 
 	require.Equal(t, 4, cps.peerSelectors[0].downloadFailures)
 	require.Equal(t, 0, cps.peerSelectors[1].downloadFailures)
+
+	// make sure successful download decreases download failures
+	cps.rankPeer(mockP1WrappedPeer, durationRank)
+	require.Equal(t, 3, cps.peerSelectors[0].downloadFailures)
 }

catchup/fetcher_test.go

@@ -287,9 +287,8 @@ func (p *testUnicastPeer) Request(ctx context.Context, tag protocol.Tag, topics
 func (p *testUnicastPeer) Respond(ctx context.Context, reqMsg network.IncomingMessage, outMsg network.OutgoingMessage) (e error) {
 
 	hashKey := uint64(0)
-	channel, found := p.responseChannels[hashKey]
-	if !found {
-	}
+	require.Contains(p.t, p.responseChannels, hashKey)
+	channel := p.responseChannels[hashKey]
 
 	select {
 	case channel <- &network.Response{Topics: outMsg.Topics}: