Disable recovered_fp_btc_sk validation in SlashedBtcDelegation

[gusin13]

The Slashed Delegation IBC packet from Babylon -> Consumer expects to have FP SK, imo this is not needed in the contract side as it has nothing to do with the extracted key.

For now i have disabled the validation which happens on packet receive, in later pr i'll need to fix the proto files both in babylon side and contract side.

Ref https://github.com/babylonlabs-io/babylon/blob/7942dae93db7f61539e4f84ecc3ff67f32299f44/proto/babylon/btcstaking/v1/packet.proto#L136

[SebastianElvis]

Isn't this needed for the consumer chain to verify the corresponding FP is indeed slashed?

[maurolacy]

Isn't this needed for the consumer chain to verify the corresponding FP is indeed slashed?

No if we trust Babylon.

[maurolacy]

If we remove this, which I'm OK with, let's just remove the commented code, and the field from the struct as well.

[maurolacy]

Nit: remove commented code.

[SebastianElvis]

Then how about we make this field optional, and then only deal with it when full-validation feature is enabled?

Asking because by default, Babylon cannot make assumption on whether the consumer side trusts itself, so Babylon has to send over the extracted secret key in any case

[gusin13]

@SebastianElvis you are right, just found this is used here

babylon-contract/contracts/btc-staking/src/validation/mod.rs

Lines 493 to 518 in 6ab5929

	#[cfg(feature = "full-validation")]
	{
	/*
	check if the SK corresponds to a FP PK that the delegation restakes to
	*/
	// get the slashed FP's SK
	let slashed_fp_sk = hex::decode(&slashed_fp_sk_hex)
	.map_err(|e| ContractError::SecP256K1Error(e.to_string()))?;
	let slashed_fp_sk = SigningKey::from_bytes(&slashed_fp_sk)
	.map_err(|e| ContractError::SecP256K1Error(e.to_string()))?;
	// calculate the corresponding VerifyingKey
	let slashed_fp_pk = slashed_fp_sk.verifying_key();
	let slashed_fp_pk_hex = hex::encode(slashed_fp_pk.to_bytes());
	// check if the PK corresponds to a FP PK that the delegation restakes to
	if !active_delegation
	.fp_btc_pk_list
	.contains(&slashed_fp_pk_hex)
	{
	return Err(ContractError::FinalityProviderNotFound(
	slashed_fp_pk_hex.to_string(),
	));
	}
	}

now qq - I was thinking keeping this field optional on the contract side still wouldn't help much as we can't turn on/off this field from Babylon side based on feature flag. Babylon would always need to send this field.

Why don't we simply send slashing evidence from Babylon, if the full-validation ff is turned on contract is responsible to extract the SK from evidence, this would avoid the extraction logic in Babylon (which in my understanding is bit slow)?

[SebastianElvis]

Why don't we simply send slashing evidence from Babylon, if the full-validation ff is turned on contract is responsible to extract the SK from evidence, this would avoid the extraction logic in Babylon (which in my understanding is bit slow)?

Yup this works as well, and it seems better as Babylon does less work 👍

[maurolacy]

Why don't we simply send slashing evidence from Babylon, if the full-validation ff is turned on contract is responsible to extract the SK from evidence, this would avoid the extraction logic in Babylon (which in my understanding is bit slow)?

This is a good idea in principle. The only problem is, that at that point the evidence might not be available anymore, as this slashing messages comes from Babylon, after slashing propagation / cascading.

Ah, you mean, sending the evidence along with the extracted SK, so that the Consumer can verify it. That makes sense.
And then, we put the verification under the full-validation feature flag.

[gusin13]

Hi guys as per today's (17 Sept) integration meeting
https://babylonlabs.atlassian.net/wiki/spaces/BABYLON/pages/32670523/2024-09-17+PoS+integration+meeting

it was discussed Babylon will send SK in the IBC packet. I will request to merge this PR as it is b/c in my cascading slashing PR I am not sending SK in IBC packet and I need the contract to disable this validation so e2e tests pass.

I'll prefer to do the discussed change in a separate PR, have made a ticket for this.
babylonlabs-io/babylon#76