Use the correct role and encoding for sync committee contributions

[dknopik]

Issue Addressed

• closes Sync committee aggregations #263

Proposed Changes

Use the correct Role for sync committee aggregations. I think I got confused here during implementation.

Additionally, I realized Go-SSV does not encode the data correctly, so I introduced a wrapper that treats the contribution as variable length to fix the encoding

[Copilot]

Pull Request Overview

This PR fixes the role used when collecting sync committee contributions by updating the role from Aggregator to SyncCommittee and adjusts related error handling and serialization. Key changes include:

• Replacing VariableList with a Contributions alias based on ContributionWrapper.
• Updating error mapping to include DecodeError details.
• Changing the signature version from ForkName::Base to ForkName::Altair and updating the role to SyncCommittee.

Reviewed Changes

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

File	Description
anchor/validator_store/src/lib.rs	Updated sync committee role usage, contributed signature error mapping, and version change.
anchor/common/ssv_types/src/consensus.rs	Introduced ContributionWrapper with custom SSZ (de)serialization and Contributions alias.

Comments suppressed due to low confidence (1)

anchor/validator_store/src/lib.rs:1402

• Please confirm that using ForkName::Altair here is intentional, ensuring that the sync committee contributions are aligned with the correct protocol version.

                        version: ForkName::Altair.into(),

[mergify]

Some required checks have failed. Could you please take a look @dknopik? 🙏

[diegomrsantos]

Could you also explain why, from a technical perspective, this is necessary? Wasn't the previous code working correctly?

[dknopik]

@diegomrsantos The encoding mismatched. First, let me explain some background info.

Basically, the way SSZ encoding works, is by having a "fixed" part of an encoded container, and a "variable" part. In the fixed part, all values (for example integers, or nested containers) with a fixed size are contained, as well as offsets pointing to the starts of all variable length values (such as a byte string of variable length).

So, if we have a list of variable length items, we have a bunch of offsets, followed by the actual items. Basically, for a list with two variable length items, it looks like this:

offset1 |offset2 |data1   |data2
08000000|0b000000|abcdef00|12323434

Here, offset1 is the little endian encoding of 8, so it is pointing to the start of data1, and offset2 is 12, pointing to the start of data2.

For a list of fixed sized values, no offsets are necessary: as the values are of fixed size, we can just divide the length of the message by the length of a single value to determine the number of items.

So much for the background. This is hard to explain well in a nutshell here, for more info ping me or take a look at the spec. Now to our issue here:

Contribution<E> has a fixed lengh. So a VariableList<Contribution<E>, U13> needs no offsets.

I noticed that Go-SSV incorrectly sends lists with offsets - basically encoding the list as if Contribution<E> had a variable length. The wrapper with the custom impls simply sets the length of a Contribution<E> to be variable, in order to have VariableList add the offsets. Note that the encoding of a single Contribution<E> is not affected by this.

Unfortunately this is needed, as we can't simply fix this on SSV side, as such a fix would be a breaking protocol change.

[diegomrsantos]

Thanks for the explanation. But how was the code behaving before this PR?

[dknopik]

It could not decode the message, causing sync committee aggregations to fail