BLS: Fix `is_infinity` flag when aggregating onto empty AggregateSignature

[jimmygchen]

Description

Original PR: This potential bug was identified by @Galoretka but the PR went stale and was never reviewed / merged.

• bls: fix is_infinity when aggregating onto empty AggregateSignature #8013

I haven't confirmed the validity of the bug but thought it's worth investigating this further.

Analysis by 🤖 (may not be accurate)

The is_infinity flag in AggregateSignature becomes inconsistent when aggregating an infinity signature onto an empty aggregate. This happens because the aggregation methods use conjunction (&&) to update the flag instead of checking if self is empty first.

Current behavior:

1. Start with AggregateSignature::empty() → point: None, is_infinity: false
2. Aggregate an infinity signature onto it
3. Result: point is now infinity, but is_infinity flag remains false

This breaks eth_fast_aggregate_verify for the edge case of empty pubkeys, causing the signature check at line 205 to fail when it should pass.

Impact: Low severity. Lighthouse doesn't trigger this code path in production because:

• Block production uses SyncAggregate::new() which starts from infinity(), not empty()
• Deserialization correctly sets the flag based on serialized bytes
• The bug only affects in-memory aggregation starting from empty()

This could affect external tools using Lighthouse's BLS library incorrectly or cause interoperability issues.

Code References

The bug exists in two methods:

add_assign method:

lighthouse/crypto/bls/src/generic_aggregate_signature.rs

Lines 125 to 136 in 713e477

	pub fn add_assign(&mut self, other: &GenericSignature<Pub, Sig>) {
	if let Some(other_point) = other.point() {
	self.is_infinity = self.is_infinity && other.is_infinity;
	if let Some(self_point) = &mut self.point {
	self_point.add_assign(other_point)
	} else {
	let mut self_point = AggSig::infinity();
	self_point.add_assign(other_point);
	self.point = Some(self_point)
	}
	}
	}

add_assign_aggregate method:

lighthouse/crypto/bls/src/generic_aggregate_signature.rs

Lines 139 to 150 in 713e477

	pub fn add_assign_aggregate(&mut self, other: &Self) {
	if let Some(other_point) = other.point() {
	self.is_infinity = self.is_infinity && other.is_infinity;
	if let Some(self_point) = &mut self.point {
	self_point.add_assign_aggregate(other_point)
	} else {
	let mut self_point = AggSig::infinity();
	self_point.add_assign_aggregate(other_point);
	self.point = Some(self_point)
	}
	}
	}

Steps to resolve

Update both add_assign and add_assign_aggregate methods to check if self.point.is_none() before updating is_infinity:

• When self is empty (point.is_none()), set is_infinity = other.is_infinity
• Otherwise, keep the existing conjunction behavior: is_infinity = self.is_infinity && other.is_infinity

This aligns with the documented behavior that empty() acts as "set to infinity before aggregation" and ensures the flag stays consistent with the underlying point.

Testing: Add a test case that:

1. Creates an AggregateSignature::empty()
2. Aggregates an infinity signature onto it
3. Verifies is_infinity() returns true
4. Verifies eth_fast_aggregate_verify with empty pubkeys returns true

Additional Info

This is a correctness issue rather than a critical mainnet bug. Fixing it improves robustness for edge cases and external library usage.

[michaelsproul]

Should be a spec test as well (if it is a real problem)