Skip to content

Commit

Permalink
fix(types: don't log all txs in block (#764)
Browse files Browse the repository at this point in the history
  • Loading branch information
lklimek authored Mar 18, 2024
1 parent da52026 commit 0bbc2f4
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 8 deletions.
10 changes: 10 additions & 0 deletions libs/log/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ func (tw *TestingLogger) AssertMatch(re *regexp.Regexp) {
tw.Logger = tw.Logger.Level(zerolog.DebugLevel)
}

// AssertContains defines assertions to check for each subsequent
// log item. It must be called before the log is generated.
// Assertion will pass if at least one log contains `s`.
//
// Note that assertions are only executed on logs matching defined log level.
// Use NewTestingLoggerWithLevel(t, zerolog.LevelDebugValue) to control this.
func (tw *TestingLogger) AssertContains(s string) {
tw.AssertMatch(regexp.MustCompile(regexp.QuoteMeta(s)))
}

// Run implements zerolog.Hook.
// Execute all log assertions against a message.
func (tw *TestingLogger) checkAssertions(msg string) {
Expand Down
7 changes: 3 additions & 4 deletions types/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,9 @@ func (b *Block) MarshalZerologObject(e *zerolog.Event) {

e.Interface("header", b.Header)
e.Interface("core_chain_lock", b.CoreChainLock)
e.Interface("data", b.Data)
e.Object("data", &b.Data)
e.Interface("evidence", b.Evidence)
e.Interface("last_commit", b.LastCommit)
e.Object("last_commit", b.LastCommit)
}

// FromProto sets a protobuf Block to the given pointer.
Expand Down Expand Up @@ -1062,8 +1062,7 @@ func (data *Data) MarshalZerologObject(e *zerolog.Event) {
e.Bool("nil", false)

e.Str("hash", data.Hash().ShortString())
e.Int("num_txs", len(data.Txs))
e.Array("txs", &data.Txs)
e.Object("txs", data.Txs)
}

// DataFromProto takes a protobuf representation of Data &
Expand Down
31 changes: 31 additions & 0 deletions types/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/dashpay/tenderdash/crypto/bls12381"
"github.com/dashpay/tenderdash/crypto/merkle"
tmbytes "github.com/dashpay/tenderdash/libs/bytes"
"github.com/dashpay/tenderdash/libs/log"
tmrand "github.com/dashpay/tenderdash/libs/rand"
tmtime "github.com/dashpay/tenderdash/libs/time"
tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
Expand Down Expand Up @@ -213,6 +214,36 @@ func TestBlockSize(t *testing.T) {
}
}

// Given a block with more than `maxLoggedTxs` transactions,
// when we marshal it for logging,
// then we should see short hashes of the first `maxLoggedTxs` transactions in the log message, ending with "..."
func TestBlockMarshalZerolog(t *testing.T) {
ctx := context.Background()
logger := log.NewTestingLogger(t)

txs := make(Txs, 0, 2*maxLoggedTxs)
expectTxs := make(Txs, 0, maxLoggedTxs)
for i := 0; i < 2*maxLoggedTxs; i++ {
txs = append(txs, Tx(fmt.Sprintf("tx%d", i)))
if i < maxLoggedTxs {
expectTxs = append(expectTxs, txs[i])
}
}

block := MakeBlock(1, txs, randCommit(ctx, t, 1, RandStateID()), nil)

// define assertions
expected := fmt.Sprintf(",\"txs\":{\"num_txs\":%d,\"hashes\":[", 2*maxLoggedTxs)
for i := 0; i < maxLoggedTxs; i++ {
expected += "\"" + expectTxs[i].Hash().ShortString() + "\","
}
expected += "\"...\"]}"
logger.AssertContains(expected)

// execute test
logger.Info("test block", "block", block)
}

func TestBlockString(t *testing.T) {
assert.Equal(t, "nil-Block", (*Block)(nil).String())
assert.Equal(t, "nil-Block", (*Block)(nil).StringIndented(""))
Expand Down
17 changes: 13 additions & 4 deletions types/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (
tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
)

// maxLoggedTxs is the maximum number of transactions to log in a Txs object.
const maxLoggedTxs = 20

// Tx is an arbitrary byte array.
// NOTE: Tx has no types at this level, so when wire encoded it's just length-prefixed.
// Might we want types here ?
Expand Down Expand Up @@ -105,20 +108,26 @@ func (txs Txs) ToSliceOfBytes() [][]byte {
return txBzs
}

func (txs *Txs) MarshalZerologArray(e *zerolog.Array) {
func (txs Txs) MarshalZerologArray(e *zerolog.Array) {
if txs == nil {
return
}

for i, tx := range *txs {
e.Str(tx.Hash().ShortString())
if i >= 20 {
for i, tx := range txs {
if i >= maxLoggedTxs {
e.Str("...")
return
}

e.Str(tx.Hash().ShortString())
}
}

func (txs Txs) MarshalZerologObject(e *zerolog.Event) {
e.Int("num_txs", len(txs))
e.Array("hashes", txs)
}

// TxRecordSet contains indexes into an underlying set of transactions.
// These indexes are useful for validating and working with a list of TxRecords
// from the PrepareProposal response.
Expand Down

0 comments on commit 0bbc2f4

Please sign in to comment.