Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce Electra boilerplate #5939

Open
dapplion opened this issue Jun 17, 2024 · 2 comments
Open

Reduce Electra boilerplate #5939

dapplion opened this issue Jun 17, 2024 · 2 comments

Comments

@dapplion
Copy link
Collaborator

dapplion commented Jun 17, 2024

Description

Electra's new attestation type has introduced a match boilerplate.

match self {
Self::Base(body) => body.attestations.len(),
Self::Altair(body) => body.attestations.len(),
Self::Bellatrix(body) => body.attestations.len(),
Self::Capella(body) => body.attestations.len(),
Self::Deneb(body) => body.attestations.len(),
Self::Electra(body) => body.attestations.len(),
}

BeaconBlockBodyRefMut::Base(ref mut blk) => {
blk.attester_slashings
.push(attester_slashing.as_base().unwrap().clone())
.expect("should update attester slashing");
}
BeaconBlockBodyRefMut::Altair(ref mut blk) => {
blk.attester_slashings
.push(attester_slashing.as_base().unwrap().clone())
.expect("should update attester slashing");
}
BeaconBlockBodyRefMut::Bellatrix(ref mut blk) => {
blk.attester_slashings
.push(attester_slashing.as_base().unwrap().clone())
.expect("should update attester slashing");
}
BeaconBlockBodyRefMut::Capella(ref mut blk) => {
blk.attester_slashings
.push(attester_slashing.as_base().unwrap().clone())
.expect("should update attester slashing");
}
BeaconBlockBodyRefMut::Deneb(ref mut blk) => {
blk.attester_slashings
.push(attester_slashing.as_base().unwrap().clone())
.expect("should update attester slashing");
}
BeaconBlockBodyRefMut::Electra(ref mut blk) => {
blk.attester_slashings
.push(attester_slashing.as_electra().unwrap().clone())
.expect("should update attester slashing");
}
}

In previous issues, I stressed how we want fork addition to be as easy as possible to foster innovation. Having to add 1000 lines of mindless code just to develop a new feature is not optimal.

We can use some macro magic to prevent a new ForkName::EIP9999 variant from having to modify all these statements.

@michaelsproul
Copy link
Member

The example shown could use a superstruct mapping macro

@michaelsproul
Copy link
Member

    pub fn attestations_len(self) -> usize {
        map_beacon_block_body_ref!(&'a _, self, |inner, cons| {
            let n = inner.attestations.len();
            cons(inner);
            n
        })
    }

Needs the cons(inner) trick to do type hinting. That's an open issue: sigp/superstruct#31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants