VC logs `Successfully published attestations` even when no attestations were published

[michaelsproul]

Description

In the VC we collect only the successful signing attempts here:

lighthouse/validator_client/src/attestation_service.rs

Lines 426 to 431 in b5bae6e

	// Execute all the futures in parallel, collecting any successful results.
	let (ref attestations, ref validator_indices): (Vec<_>, Vec<_>) = join_all(signing_futures)
	.await
	.into_iter()
	.flatten()
	.unzip();

If all signings fail due to slashing protection checks, timeouts or other errors, then that Vec of attestations will be empty.

Despite the emptiness Lighthouse will still POST the empty list to the BN and log a success message indicating that 0 attestations were published:

Feb 24 04:40:52.001 CRIT Not signing slashable attestation error: InvalidAttestation(DoubleVote(SignedAttestation { source_epoch: Epoch(33454), target_epoch: Epoch(33456), signing_root: SigningRoot(0xb2bcf61564c960b08be6e453555aafb24e03024cd77f45e2ff54374e42493d9d) })), attestation: AttestationData { slot: Slot(1070604), index: 6, beacon_block_root: 0x55e45f0439135f0459541aa16cddd389300d9b8200674bc5affa3edb6dba07d8, source: Checkpoint { epoch: Epoch(33455), root: 0x11b52805a108360db7f5e2330ebb05a94aa9a95d0b903be737871daecf9c6d7a }, target: Checkpoint { epoch: Epoch(33456), root: 0xeb2df0e20834b06219194bc4f435f390233375ab8a3be62e83e3fafcacb14ac2 } }
Feb 24 04:40:52.002 CRIT Failed to sign attestation slot: 1070604, committee_index: 6, validator: 0xa786f5baa59d30f95ace28e7a3f90c3300e33f3a48f2da9db89bba1d3b6fef2c94a39173735fccea87f6c507c6605698, error: Slashable(InvalidAttestation(DoubleVote(SignedAttestation { source_epoch: Epoch(33454), target_epoch: Epoch(33456), signing_root: SigningRoot(0xb2bcf61564c960b08be6e453555aafb24e03024cd77f45e2ff54374e42493d9d) }))), service: attestation
Feb 24 04:40:52.002 INFO Successfully published attestations type: unaggregated, slot: 1070604, committee_index: 6, head_block: 0x55e45f0439135f0459541aa16cddd389300d9b8200674bc5affa3edb6dba07d8, validator_indices: [], count: 0, service: attestation

Version

Lighthouse v5.0.0

Steps to resolve

1. Don't POST if the attestations list is empty
2. Log another message like WARN No attestations were published

[pahor167]

I would like to look into this issue, if possible

[michaelsproul]

Please do @pahor167, I can review!

[kamuik16]

Please do @pahor167, I can review!

hey @michaelsproul, I opened a PR for this issue #5318. Please review and suggest changes.

[pahor167]

Please do @pahor167, I can review!

hey @michaelsproul, I opened a PR for this issue #5318. Please review and suggest changes.

I got a bit stuck on writing test, but I can see now that it might not be necessary. @michaelsproul is there any rule of thumb when tests should be added and when it is ok publish a PR without it?

[kamuik16]

Please do @pahor167, I can review!

hey @michaelsproul, I opened a PR for this issue #5318. Please review and suggest changes.

I got a bit stuck on writing test, but I can see now that it might not be necessary. @michaelsproul is there any rule of thumb when tests should be added and when it is ok publish a PR without it?

Ohh, I am sorry @pahor167, I saw that you were assigned the issue two days ago and the fix was simple so I thought you may have skipped the issue.

[jimmygchen]

Fixed in 5318 🎉 and will be included in v5.1.0