BFT synch: add block to decision function

[tock-ibm]

Needed for BFT synch integration

#35

[Copilot]

Pull request overview

Adds a helper to convert a ledger common.Block into a SmartBFT Decision, intended to support upcoming BFT synchronizer integration (#35).

Changes:

• Introduce consensus.BlockToDecision(*common.Block) *smartbft_types.Decision for decoding decision payloads from blocks.
• Add a unit test that round-trips DecisionToBytes → block → BlockToDecision.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
node/consensus/block_utils.go	Adds BlockToDecision helper for decoding a decision from block.Data.Data[0].
node/consensus/block_utils_test.go	Adds a basic happy-path test for BlockToDecision.

Comments suppressed due to low confidence (3)

node/consensus/block_utils.go:181

• The doc comment says the function returns nil if the block is "not valid", but it doesn't perform structural/hash-chain validation—only checks for presence of Data and ability to parse Data[0] as a decision. Consider rewording to reflect what is actually validated (or add validation if that’s required here).
  node/consensus/block_utils.go:191
• BlockToDecision returns nil for a range of error cases (nil block, empty data, parse error), but the BFT synchronizer path dereferences the returned pointer (e.g., Latest: *decision). This can turn a recoverable parse/validation failure into a nil-pointer panic and also makes failures hard to diagnose. Consider changing this helper to return (*types.Decision, error) (or alternatively panic/log and return a non-nil decision) so callers can handle/propagate the failure explicitly.
  node/consensus/block_utils.go:188
• This function silently ignores any additional entries in block.Data.Data and only parses index 0. If this is meant to convert a consensus-ledger decision block (which is always a single data item), it would be safer to enforce len(block.Data.Data) == 1 and treat other values as invalid to avoid accepting malformed blocks.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

BlockToDecision is documented to return nil on invalid blocks / conversion failures, but the test only covers the happy-path. Add negative test cases (nil block, nil/empty Data, garbage bytes) to lock in the nil-on-error behavior (or updated error behavior if you change the API).

Suggested change

	}
	}
	func TestBlockToDecision_InvalidInputs(t *testing.T) {
	t.Run("nil block", func(t *testing.T) {
	decision := consensus.BlockToDecision(nil)
	assert.Nil(t, decision)
	})
	t.Run("nil data field", func(t *testing.T) {
	block := &common.Block{
	Data: nil,
	}
	decision := consensus.BlockToDecision(block)
	assert.Nil(t, decision)
	})
	t.Run("empty data slice - nil", func(t *testing.T) {
	block := &common.Block{
	Data: &common.BlockData{
	Data: nil,
	},
	}
	decision := consensus.BlockToDecision(block)
	assert.Nil(t, decision)
	})
	t.Run("empty data slice - zero length", func(t *testing.T) {
	block := &common.Block{
	Data: &common.BlockData{
	Data: [][]byte{},
	},
	}
	decision := consensus.BlockToDecision(block)
	assert.Nil(t, decision)
	})
	t.Run("garbage bytes", func(t *testing.T) {
	block := &common.Block{
	Data: &common.BlockData{
	// This byte slice is not produced by state.DecisionToBytes and should
	// cause BlockToDecision to fail and return nil.
	Data: [][]byte{[]byte{0x01, 0x02, 0x03}},
	},
	}
	decision := consensus.BlockToDecision(block)
	assert.Nil(t, decision)
	})
	}

[HagarMeir]

why change the file name?
‎node/consensus/utils.go‎ --> ‎node/consensus/block_utils.go
it is not until block utils...

[tock-ibm]

why change the file name? ‎node/consensus/utils.go‎ --> ‎node/consensus/block_utils.go it is not until block utils...

I wanted to add tests but the util_test.go actually contains utility methods for tests, no tests them selves... and the methods in block_utils all have "Block" in them ;-)

I can change back and rename util_test.go to setup_test.go