Submit Fiat-Shamir

[yrong]

A prototype implementation based on the spec from Bhargav. Great thanks for that.

Resolves: https://linear.app/snowfork/issue/SNO-1489

Requires: #1457

[codecov]

Codecov Report

Attention: Patch coverage is 67.18750% with 21 lines in your changes missing coverage. Please review.

Project coverage is 79.82%. Comparing base (6993ef4) to head (b844164).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
contracts/src/BeefyClient.sol	67.18%	18 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1462      +/-   ##
==========================================
- Coverage   80.21%   79.82%   -0.39%     
==========================================
  Files          20       20              
  Lines         854      917      +63     
  Branches      120      160      +40     
==========================================
+ Hits          685      732      +47     
- Misses        146      159      +13     
- Partials       23       26       +3     

Flag	Coverage Δ
solidity	79.82% <67.18%> (-0.39%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[acatangiu]

I believe you also need some mechanism to reset the interactive BEEFY protocol if it was in progress when a Fiat-Shamir-based BEEFY submission is accepted by the light-client.

also, make sure to do security audit for this feature :)

[yrong]

need some mechanism to reset the interactive BEEFY protocol if it was in progress when a Fiat-Shamir-based BEEFY submission is accepted by the light-client.

Though I'm not sure the reset is nessessary, as we won't accept an obsolete commitment anyway.

snowbridge/contracts/src/BeefyClient.sol

Lines 773 to 776 in 2d55aa0

	if (commitment.blockNumber <= latestBeefyBlock) {
	// ticket is obsolete
	revert StaleCommitment();
	}

[yrong]

forge test --match-path test/BeefyClient.t.sol -vvv --gas-report

Function Name	Min	Avg	Median	Max	# Calls
commitPrevRandao	24200	43393	46808	46808	22
submitFinal	232936	361220	249643	797277	17
submitInitial	34704	113748	130279	132558	29
submitFiatShamir	1766534	1872289	1872289	1978045	2
...

Gas cost of PreRandao submission with 28 signatures across 3 calls:
132558 + 46808 + 797277 = 976643

Gas cost of submitFiatShamir with 101 signatures:
1978045

[bhargavbh]

overall LGTM. Probably we write some tests for scenario when the relayers are calling fiatShamir and interactive random sampling concurrently. In theory, the staleCommitment check in the verifier should suffice.

[bhargavbh]

if we are to rely on relative security to bitcoin mining then we have to use the same hash function sha256 when generating the fiatshamir hash (instead of keccak256 here).

[bhargavbh]

using keccak256 for bitFieldHash and commitmentHash is fine

[yrong]

ec230b8

[bhargavbh]

@yrong just realised that bitcoin mining uses double-SHA256 (sha256d), and there is no sha256d precompile for evm. Hence, to compute fiatShamirHash, we need to hash the output of the first hash again, i.e., sha256(sha256(concatenated_bytes)).

[yrong]

Good to know! Fixed in b94851e

[bhargavbh]

we have 2/3rd +1 honesty assumption on polkadot validators. Hence it is sufficient to check 1/3rd +1 validator signatures to ensure at least 1 honest validator signed the payload. We can get away with min(fiatShamirRequiredSignature, n/3 +1) here. This also sufficient for the random sampling protocol.

[yrong]

e277eb7

[vgeddes]

This should be immutable and initialized via the constructor.

[yrong]

dcc7588