Migrating eip-3076

[nadtech-hub]

This should close #7894

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[michaelsproul]

This PR doesn't accomplish the desired goal. You've moved all of the slashing_protection crate into eip_3076, but what we would ideally do, is just extract the "pure" EIP-3076 code (without database dependencies) into eip_3076, while leaving all of the database code and Lighthouse-specific logic in the slashing_protection crate.

This would allow us to reduce the dependency surface of crates like eth2 which only need the "pure" EIP-3076 code, and should not depend on Lighthouse's implementation details (our choice of sqlite for the slashing protection database).

[michaelsproul]

The whole interchange-tests directory should not be committed. It was part of the .gitignore in slashing_protection, and should continue to be ignored.

The source lives here:

• https://github.com/eth-clients/slashing-protection-interchange-tests

[nadtech-hub]

This PR doesn't accomplish the desired goal. You've moved all of the slashing_protection crate into eip_3076, but what we would ideally do, is just extract the "pure" EIP-3076 code (without database dependencies) into eip_3076, while leaving all of the database code and Lighthouse-specific logic in the slashing_protection crate.

This would allow us to reduce the dependency surface of crates like eth2 which only need the "pure" EIP-3076 code, and should not depend on Lighthouse's implementation details (our choice of sqlite for the slashing protection database).

Makes sense, I'll do changes accordingly

[michaelsproul]

Thanks!

Let us know if you have any questions, some of the stuff might be tricky to extract

[nadtech-hub]

Thanks!

Let us know if you have any questions, some of the stuff might be tricky to extract

It was matter of migrating interchange, signed_block and signed_attestation and corresponding tests, wasn't it? For some reason, I moved the whole thing)

[michaelsproul]

I think just moving the interchange and its immediate dependents is the way to go, yeah.

[michaelsproul]

This is the wrong SignedAttestation. You should move this one:

lighthouse/validator_client/slashing_protection/src/interchange.rs

Lines 36 to 46 in fd10b63

	#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
	#[serde(deny_unknown_fields)]
	#[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
	pub struct SignedAttestation {
	#[serde(with = "serde_utils::quoted_u64::require_quotes")]
	pub source_epoch: Epoch,
	#[serde(with = "serde_utils::quoted_u64::require_quotes")]
	pub target_epoch: Epoch,
	#[serde(skip_serializing_if = "Option::is_none")]
	pub signing_root: Option<Hash256>,
	}

.

[michaelsproul]

Also the wrong SignedBlock.

[michaelsproul]

I don't think Safe, NotSafe or SigningRoot need to be here either. Just move the Interchange type. Start by moving interchange.rs

[nadtech-hub]

I think just moving the interchange and its immediate dependents is the way to go, yeah.

I also thought that interchange file format needs be moved out along with inner data structures. Separating interchange from database is cumbersome

[michaelsproul]

The interchange format is the main thing we need to pull out, even if it's just the type definition. Can you elaborate on the interaction with the database that makes it hard to extract?

[nadtech-hub]

The interchange format is the main thing we need to pull out, even if it's just the type definition. Can you elaborate on the interaction with the database that makes it hard to extract?

Interchange test modules are harder to migrate because of cohesion with db.
Interchange structure is extractable as a whole but tests will remain in slashing_protection.
Some data structures are replicated in interchange.

[michaelsproul]

Yeah that sounds acceptable. The main thing we're trying to accomplish is pulling out the type definitions so we can remove the dependency of the eth2 crate on slashing_protection. Instead eth2 should depend on eip_3076.