diff --git a/blockchain/standalone/error.go b/blockchain/standalone/error.go index 0ebec059..152d3f4c 100644 --- a/blockchain/standalone/error.go +++ b/blockchain/standalone/error.go @@ -20,13 +20,9 @@ const ( // lower than the required target difficultly. ErrHighHash = ErrorKind("ErrHighHash") - // ErrTSpendStartInvalidExpiry indicates that an invalid expiry was - // provided to calculate the start of a treasury spend vote window. - ErrTSpendStartInvalidExpiry = ErrorKind("ErrTSpendStartInvalidExpiry") - - // ErrTSpendEndInvalidExpiry indicates that an invalid expiry was - // provided to calculate the end of a treasury spend vote window. - ErrTSpendEndInvalidExpiry = ErrorKind("ErrTSpendEndInvalidExpiry") + // ErrInvalidTSpendExpiry indicates that an invalid expiry was + // provided when calculating the treasury spend voting window. + ErrInvalidTSpendExpiry = ErrorKind("ErrInvalidTSpendExpiry") ) // Error satisfies the error interface and prints human-readable errors. diff --git a/blockchain/standalone/error_test.go b/blockchain/standalone/error_test.go index 1ed1598a..f3f88346 100644 --- a/blockchain/standalone/error_test.go +++ b/blockchain/standalone/error_test.go @@ -18,8 +18,7 @@ func TestErrorKindStringer(t *testing.T) { }{ {ErrUnexpectedDifficulty, "ErrUnexpectedDifficulty"}, {ErrHighHash, "ErrHighHash"}, - {ErrTSpendStartInvalidExpiry, "ErrTSpendStartInvalidExpiry"}, - {ErrTSpendEndInvalidExpiry, "ErrTSpendEndInvalidExpiry"}, + {ErrInvalidTSpendExpiry, "ErrInvalidTSpendExpiry"}, } t.Logf("Running %d tests", len(tests)) diff --git a/blockchain/standalone/treasury.go b/blockchain/standalone/treasury.go index 70bcd153..bd401fe8 100644 --- a/blockchain/standalone/treasury.go +++ b/blockchain/standalone/treasury.go @@ -24,26 +24,29 @@ func IsTreasuryVoteInterval(height, tvi uint64) bool { return height%tvi == 0 && height != 0 } -// CalculateTSpendWindowStart calculates the start of a treasury voting window -// based on the parameters that are passed. Great care must be taken to ensure -// this function is only called with an expiry that *IS* on a TVI. -func CalculateTSpendWindowStart(expiry uint32, tvi, multiplier uint64) (uint32, error) { - if !IsTreasuryVoteInterval(uint64(expiry-2), tvi) { - return 0, ruleError(ErrTSpendStartInvalidExpiry, - fmt.Sprintf("invalid start expiry: %v", expiry)) +// CalcTSpendWindow calculates the start and end of a treasury voting window +// based on the parameters that are passed. An error will be returned if the +// provided expiry is not two more than a treasury vote interval (TVI) or before +// a single voting window is possible. +func CalcTSpendWindow(expiry uint32, tvi, multiplier uint64) (uint32, uint32, error) { + // Ensure the provided expiry is at least higher than a single voting + // window. + minReqExpiry := tvi*multiplier + 2 + if uint64(expiry) < minReqExpiry { + str := fmt.Sprintf("expiry %d must be at least %d for the voting "+ + "window defined by a TVI of %d with a multiplier of %d", expiry, + minReqExpiry, tvi, multiplier) + return 0, 0, ruleError(ErrInvalidTSpendExpiry, str) } - return expiry - uint32(tvi*multiplier) - 2, nil -} -// CalculateTSpendWindowEnd calculates the end of a treasury voting window -// based on the parameters that are passed. Great care must be taken to ensure -// this function is only called with an expiry that *IS* on a TVI. -func CalculateTSpendWindowEnd(expiry uint32, tvi uint64) (uint32, error) { + // Ensure the provided expiry is two more than a TVI. if !IsTreasuryVoteInterval(uint64(expiry-2), tvi) { - return 0, ruleError(ErrTSpendEndInvalidExpiry, - fmt.Sprintf("invalid end expiry: %v", expiry)) + str := fmt.Sprintf("expiry %d must be two more than a multiple of the "+ + "treasury vote interval %d", expiry, tvi) + return 0, 0, ruleError(ErrInvalidTSpendExpiry, str) } - return expiry - 2, nil + + return expiry - uint32(tvi*multiplier) - 2, expiry - 2, nil } // InsideTSpendWindow returns true if the provided block height is inside the @@ -55,13 +58,10 @@ func CalculateTSpendWindowEnd(expiry uint32, tvi uint64) (uint32, error) { // Note: The end is INCLUSIVE in order to determine if a TSPEND is allowed in a // block despite the fact that voting window is EXCLUSIVE. func InsideTSpendWindow(blockHeight int64, expiry uint32, tvi, multiplier uint64) bool { - s, err := CalculateTSpendWindowStart(expiry, tvi, multiplier) + start, end, err := CalcTSpendWindow(expiry, tvi, multiplier) if err != nil { return false } - e, err := CalculateTSpendWindowEnd(expiry, tvi) - if err != nil { - return false - } - return uint32(blockHeight) >= s && uint32(blockHeight) <= e + + return uint32(blockHeight) >= start && uint32(blockHeight) <= end } diff --git a/blockchain/standalone/treasury_test.go b/blockchain/standalone/treasury_test.go index 9f727cf9..5cb081bd 100644 --- a/blockchain/standalone/treasury_test.go +++ b/blockchain/standalone/treasury_test.go @@ -9,19 +9,106 @@ import ( "testing" ) -func TestTSpendExpiryNegative(t *testing.T) { - // 5 is not a valid start for a tvi of 11 with a mul of 3. - _, err := CalculateTSpendWindowStart(5, 11, 3) - if !errors.Is(err, ErrTSpendStartInvalidExpiry) { - t.Fatalf("expected %v got %v", - ErrTSpendStartInvalidExpiry, err) - } +const ( + // mainNetTVI is the treasury vote interval for mainnet. + mainNetTVI = 288 - // 5 is not a valid end for a tvi of 11. - _, err = CalculateTSpendWindowEnd(5, 11) - if !errors.Is(err, ErrTSpendEndInvalidExpiry) { - t.Fatalf("expected %v got %v", - ErrTSpendEndInvalidExpiry, err) + // mainNetTVIMul is the treasury vote interval multiplier for mainnet. + mainNetTVIMul = 12 +) + +// TestCalcTSpendWindow ensures that the function that calculates the start and +// end of a treasury spend voting window returns the expected results including +// error conditions. +func TestCalcTSpendWindow(t *testing.T) { + tests := []struct { + name string // test description + expiry uint32 // expiry to calculate the window for + tvi uint64 // treasury vote interval + tvimul uint64 // treasury vote interval multiplier + err error // expected error + wantStart uint32 // expected start result + wantEnd uint32 // expected end result + }{{ + name: "zero is not a valid expiry", + expiry: 0, + tvi: mainNetTVI, + tvimul: mainNetTVIMul, + err: ErrInvalidTSpendExpiry, + wantStart: 0, + wantEnd: 0, + }, { + name: "min required expiry - 1", + expiry: mainNetTVI*mainNetTVIMul + 1, + tvi: mainNetTVI, + tvimul: mainNetTVIMul, + err: ErrInvalidTSpendExpiry, + wantStart: 0, + wantEnd: 0, + }, { + name: "not a TVI + 2", + expiry: mainNetTVI*mainNetTVIMul + 3, + tvi: mainNetTVI, + tvimul: mainNetTVIMul, + err: ErrInvalidTSpendExpiry, + wantStart: 0, + wantEnd: 0, + }, { + name: "5 is not a valid start or end for a tvi 11, mul 3", + expiry: 5, + tvi: 11, + tvimul: 3, + err: ErrInvalidTSpendExpiry, + wantStart: 0, + wantEnd: 0, + }, { + name: "first possible valid mainnet params", + expiry: mainNetTVI*mainNetTVIMul + 2, + tvi: mainNetTVI, + tvimul: mainNetTVIMul, + err: nil, + wantStart: 0, + wantEnd: mainNetTVI * mainNetTVIMul, + }, { + name: "second possible valid mainnet params", + expiry: mainNetTVI*mainNetTVIMul*2 + 2, + tvi: mainNetTVI, + tvimul: mainNetTVIMul, + err: nil, + wantStart: mainNetTVI * mainNetTVIMul, + wantEnd: mainNetTVI * mainNetTVIMul * 2, + }, { + name: "5186 for tvi 288, mul 7 is window [3168, 5184)", + expiry: 5186, + tvi: 288, + tvimul: 7, + err: nil, + wantStart: 5186 - 288*7 - 2, + wantEnd: 5186 - 2, + }} + + for _, test := range tests { + // Calculate result and ensure the expected error is produced. + start, end, err := CalcTSpendWindow(test.expiry, test.tvi, test.tvimul) + if !errors.Is(err, test.err) { + t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, + test.err) + continue + } + + // Ensure the expected start value is calculated. + if start != test.wantStart { + t.Errorf("%q: unexpected start val -- got %v, want %v", test.name, + start, test.wantStart) + continue + } + + // Ensure the expected end value is calculated. + if end != test.wantEnd { + t.Errorf("%q: unexpected end val -- got %v, want %v", test.name, + end, test.wantEnd) + continue + } } } @@ -32,32 +119,4 @@ func TestTSpendExpiry(t *testing.T) { if expiry != 5186 { t.Fatalf("got %v, expected 5186", expiry) } - start, err := CalculateTSpendWindowStart(expiry, tvi, mul) - if err != nil { - t.Fatal(err) - } - end, err := CalculateTSpendWindowEnd(expiry, tvi) - if err != nil { - t.Fatal(err) - } - - // Expect 3168 = expiry - (tvi*mul) - 2 - expectedStart := expiry - uint32(tvi*mul) - 2 - if start != expectedStart { - t.Fatalf("got %v, expected %v", start, expectedStart) - } - - // Expect 5184 = expiry - 2 - expectedEnd := expiry - 2 - if end != expectedEnd { - t.Fatalf("got %v, expected %v", end, expectedEnd) - } - - // Hard check numbers as well. - if expectedStart != 3168 { - t.Fatalf("got %v, expected %v", expectedStart, 3168) - } - if expectedEnd != 5184 { - t.Fatalf("got %v, expected %v", expectedEnd, 5184) - } } diff --git a/blockchain/treasury.go b/blockchain/treasury.go index 01be28ab..0b859a85 100644 --- a/blockchain/treasury.go +++ b/blockchain/treasury.go @@ -885,17 +885,12 @@ func (b *BlockChain) tSpendCountVotes(prevNode *blockNode, tspend *dcrutil.Tx) ( ) expiry := tspend.MsgTx().Expiry - t.start, err = standalone.CalculateTSpendWindowStart(expiry, + t.start, t.end, err = standalone.CalcTSpendWindow(expiry, b.chainParams.TreasuryVoteInterval, b.chainParams.TreasuryVoteIntervalMultiplier) if err != nil { return nil, err } - t.end, err = standalone.CalculateTSpendWindowEnd(expiry, - b.chainParams.TreasuryVoteInterval) - if err != nil { - return nil, err - } nextHeight := prevNode.height + 1 trsyLog.Tracef(" tSpendCountVotes: nextHeight %v start %v expiry %v", diff --git a/blockchain/treasury_test.go b/blockchain/treasury_test.go index 66dc0f89..54b84793 100644 --- a/blockchain/treasury_test.go +++ b/blockchain/treasury_test.go @@ -523,11 +523,7 @@ func TestTSpendVoteCount(t *testing.T) { tspendFee := 100 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) - if err != nil { - t.Fatal(err) - } - end, err := standalone.CalculateTSpendWindowEnd(expiry, tvi) + start, end, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -738,7 +734,7 @@ func TestTSpendVoteCount(t *testing.T) { // Use exact hight to validate that tspend starts on next tvi. expiry = standalone.CalculateTSpendExpiry(int64(g.Tip().Header.Height), tvi, mul) - start, err = standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err = standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -946,7 +942,7 @@ func TestTSpendEmptyTreasury(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -1170,7 +1166,7 @@ func TestTSpendExpendituresPolicy(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -1661,7 +1657,7 @@ func TestExpendituresReorg(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -1869,7 +1865,7 @@ func TestSpendableTreasuryTxs(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -2164,7 +2160,7 @@ func TestTSpendDupVote(t *testing.T) { // --------------------------------------------------------------------- nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -2306,7 +2302,7 @@ func TestTSpendTooManyTSpend(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -2423,7 +2419,7 @@ func TestTSpendWindow(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + uint32(tvi*mul*4) + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -2547,7 +2543,7 @@ func TestTSpendSignature(t *testing.T) { nextBlockHeight := g.Tip().Header.Height //+ uint32(tvi*mul*4) + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -2767,7 +2763,7 @@ func TestTSpendSignatureInvalid(t *testing.T) { nextBlockHeight := g.Tip().Header.Height //+ uint32(tvi*mul*4) + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -2965,7 +2961,7 @@ func TestTSpendExists(t *testing.T) { nextBlockHeight := g.Tip().Header.Height + 1 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -3870,7 +3866,7 @@ func TestTSpendCorners(t *testing.T) { tspendFee := 100 expiry := standalone.CalculateTSpendExpiry(int64(nextBlockHeight), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) + start, _, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -3958,11 +3954,7 @@ func TestTSpendFirstTVICorner(t *testing.T) { // total of 55 possible. That easily is voted in since 60% is 33 yes // votes. expiry := standalone.CalculateTSpendExpiry(140, tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) - if err != nil { - t.Fatal(err) - } - end, err := standalone.CalculateTSpendWindowEnd(expiry, tvi) + start, end, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } @@ -4250,11 +4242,7 @@ func TestTSpendVoteCountSynthetic(t *testing.T) { tpb := params.TicketsPerBlock expiry := standalone.CalculateTSpendExpiry(int64(tmh), tvi, mul) - start, err := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) - if err != nil { - t.Fatal(err) - } - end, err := standalone.CalculateTSpendWindowEnd(expiry, tvi) + start, end, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } diff --git a/blockchain/validate.go b/blockchain/validate.go index 00c45a72..1b0a2940 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -3432,20 +3432,15 @@ func (b *BlockChain) tspendChecks(prevNode *blockNode, block *dcrutil.Block) err return ruleError(ErrNotTVI, str) } - // Assert that the tspend is inside the correct window. + // Assert that the treasury spend is inside the correct window. exp := stx.MsgTx().Expiry if !standalone.InsideTSpendWindow(blockHeight, exp, b.chainParams.TreasuryVoteInterval, b.chainParams.TreasuryVoteIntervalMultiplier) { - s, _ := standalone.CalculateTSpendWindowStart(exp, - b.chainParams.TreasuryVoteInterval, - b.chainParams.TreasuryVoteIntervalMultiplier) - str := fmt.Sprintf("block contains TSpend "+ - "transaction (%v) that is outside "+ - "of the valid window: height %v "+ - "start %v expiry %v", - stx.Hash(), block.Height(), s, exp) + str := fmt.Sprintf("block at height %d contains treasury spend "+ + "transaction %v with expiry %v that is outside of the valid "+ + "window", blockHeight, stx.Hash(), exp) return ruleError(ErrInvalidTSpendWindow, str) } diff --git a/internal/mempool/mempool.go b/internal/mempool/mempool.go index 0b843f29..b50eeaef 100644 --- a/internal/mempool/mempool.go +++ b/internal/mempool/mempool.go @@ -1630,8 +1630,7 @@ func (mp *TxPool) maybeAcceptTransaction(tx *dcrutil.Tx, isNew, rateLimit, allow // its voting is supposed to start. We arbitrarily define as // "too far in the future" as the vote starting greater than or // equal to two full voting windows in the future. - voteStart, err := standalone.CalculateTSpendWindowStart(msgTx.Expiry, - tvi, mul) + voteStart, _, err := standalone.CalcTSpendWindow(msgTx.Expiry, tvi, mul) if err != nil { str := fmt.Sprintf("Invalid tspend expiry %d: %v ", msgTx.Expiry, err) diff --git a/internal/mempool/mempool_test.go b/internal/mempool/mempool_test.go index 87299682..22ad348f 100644 --- a/internal/mempool/mempool_test.go +++ b/internal/mempool/mempool_test.go @@ -2613,7 +2613,7 @@ func TestHandlesTSpends(t *testing.T) { // when voting ends and advance the fake chain to just before that // height. The tspend can be mined on the block the vote ends, which is // a TVI block. - endVote, err := standalone.CalculateTSpendWindowEnd(expiry, tvi) + _, endVote, err := standalone.CalcTSpendWindow(expiry, tvi, mul) if err != nil { t.Fatal(err) } diff --git a/internal/mining/mining.go b/internal/mining/mining.go index 1bb892cc..af475d88 100644 --- a/internal/mining/mining.go +++ b/internal/mining/mining.go @@ -1461,13 +1461,9 @@ mempoolLoop: exp, g.chainParams.TreasuryVoteInterval, g.chainParams.TreasuryVoteIntervalMultiplier) { - s, _ := standalone.CalculateTSpendWindowStart(exp, - g.chainParams.TreasuryVoteInterval, - g.chainParams.TreasuryVoteIntervalMultiplier) - log.Tracef("Skipping tspend %v because it is "+ - "outside of the window: height %v "+ - "start %v expiry %v reason %v", - tx.Hash(), nextBlockHeight, s, exp, err) + log.Tracef("Skipping treasury spend %v at height %d because it "+ + "has an expiry of %d that is outside of the voting window", + tx.Hash(), nextBlockHeight, exp) continue } diff --git a/internal/rpcserver/rpcserver.go b/internal/rpcserver/rpcserver.go index 3d07c144..c88646c1 100644 --- a/internal/rpcserver/rpcserver.go +++ b/internal/rpcserver/rpcserver.go @@ -3323,10 +3323,9 @@ func handleGetTreasurySpendVotes(_ context.Context, s *Server, cmd interface{}) } } - // The following errors can be ignored because we checked the - // expiry is in a TVI earlier. - start, _ := standalone.CalculateTSpendWindowStart(expiry, tvi, mul) - end, _ := standalone.CalculateTSpendWindowEnd(expiry, tvi) + // The following error can be ignored because the expiry was verified to + // be in a TVI earlier. + start, end, _ := standalone.CalcTSpendWindow(expiry, tvi, mul) votes[i] = types.TreasurySpendVotes{ Hash: txHash.String(),