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

Skip mandatory commitment when beefy authorities not changed #1215

Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
a686dfa
Permit non-consecutive increases in validator set
Lederstrumpf Jan 31, 2024
a223f97
adjust submitInitial too
Lederstrumpf Jan 31, 2024
ea2f146
Improve filtering
vgeddes May 27, 2024
ebb5794
Skip mandatory commitment
yrong Feb 4, 2024
0faba1f
Merge remote-tracking branch 'Lederstrumpf/permit-nonconsecutive-incr…
yrong May 28, 2024
5eba01b
Fix updating with current beefy state
yrong May 28, 2024
482e8a2
Ignore submit when authorities not change
yrong May 28, 2024
40f9592
Cleanup
yrong May 29, 2024
8853005
Improve BEEFY relayer
vgeddes May 30, 2024
629a478
Sync beefy commitment on demand
yrong May 31, 2024
67c52e8
Minor fix
yrong May 31, 2024
dba8cfc
More comments
yrong May 31, 2024
a886879
More refactoring
yrong May 31, 2024
fd7db5b
review feedback
vgeddes May 31, 2024
58d83bc
review feedback #2
vgeddes May 31, 2024
80b4dbd
unused code
vgeddes May 31, 2024
c11ed7d
Fix for boundary update
yrong Jun 1, 2024
10f38d4
Fix for skip mandatory commitment
yrong Jun 1, 2024
98e3e74
Some refactoring
yrong Jun 3, 2024
fc29329
Fix ci breaking
yrong Jun 3, 2024
6efd6c1
Merge branch 'vincent/beefy-relay-improvements' into ron/beefy-relay-…
yrong Jun 3, 2024
715ac5a
Find for next beefy block
yrong Jun 3, 2024
7d06d62
Merge branch 'ron/beefy-relay-improvements' into ron/skip-mandatory-c…
yrong Jun 3, 2024
16c5012
Merge branch 'ron/skip-mandatory-commitment' of https://github.com/Sn…
yrong Jun 3, 2024
913960e
Improve log
yrong Jun 3, 2024
edd9f21
Remove check unrelated
yrong Jun 3, 2024
47b0530
Merge branch 'ron/beefy-relay-improvements' into ron/skip-mandatory-c…
yrong Jun 3, 2024
08c70f1
Check commitment in CurrentValidatorSetID
yrong Jun 3, 2024
95845a3
Sync beefy commitment on demand (#1217)
yrong Jun 3, 2024
bb0c52f
Resolve conflicts
yrong Jun 3, 2024
6d26d04
unused context parameter
vgeddes Jun 3, 2024
39aac20
Merge branch 'main' into vincent/beefy-relay-improvements
vgeddes Jun 3, 2024
ba37da4
Merge branch 'vincent/beefy-relay-improvements' into ron/skip-mandato…
yrong Jun 4, 2024
3d8daa6
Test happy path
yrong Jun 4, 2024
47cceeb
More tests
yrong Jun 4, 2024
0f221e5
More tests
yrong Jun 4, 2024
ae9c1ad
Merge branch 'main' into ron/skip-mandatory-commitment
yrong Jun 4, 2024
585619e
For rococo compatibility
yrong Jun 4, 2024
4b80965
Merge branch 'main' into ron/skip-mandatory-commitment
yrong Jun 5, 2024
2b30202
Revert changes
yrong Jun 5, 2024
c99028d
Allow commitment in future
yrong Jun 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions contracts/src/BeefyClient.sol
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,12 @@ contract BeefyClient {

ValidatorSetState storage vset;
uint16 signatureUsageCount;
if (commitment.validatorSetID == currentValidatorSet.id) {
if (commitment.validatorSetID == currentValidatorSet.id || commitment.validatorSetID == nextValidatorSet.id - 1)
{
signatureUsageCount = currentValidatorSet.usageCounters.get(proof.index);
currentValidatorSet.usageCounters.set(proof.index, signatureUsageCount.saturatingAdd(1));
vset = currentValidatorSet;
} else if (commitment.validatorSetID == nextValidatorSet.id) {
} else if (commitment.validatorSetID >= nextValidatorSet.id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some tests in the solidity that skips a mandatory commitment.

Example scenario:
We are in validator set id 6 (next validator set 7), we skip 7 and 8 and then import 9. After importing 9 the current validator set will be 7 and the next validator set will be 10.

See here: https://github.com/Snowfork/snowbridge/pull/1215/files#diff-473fead6cd500a4a05f06d0cea4d9ef166c9bdbe2a2e1a06a33d2e1aedcc428eR379-R382

We also need to make sure that a future commit does not commit to a block number in the past and a test that covers it.

Context:
#1137 (comment)
#1137 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch Alistair, yeah, we need more unit tests for this work.

Copy link
Contributor Author

@yrong yrong Jun 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so in this case the current&next validatorId pair will be (7,10) and latestBeefyBlock is in session 9.

I would expect a minor change here

if (commitment.validatorSetID == currentValidatorSet.id) {

from if (commitment.validatorSetID == currentValidatorSet.id) to if (commitment.validatorSetID == currentValidatorSet.id) || commitment.validatorSetID == nextValidatorSet.id - 1 will just work but I'll definetely do more tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

signatureUsageCount = nextValidatorSet.usageCounters.get(proof.index);
nextValidatorSet.usageCounters.set(proof.index, signatureUsageCount.saturatingAdd(1));
vset = nextValidatorSet;
Expand Down Expand Up @@ -354,11 +355,12 @@ contract BeefyClient {

bool is_next_session = false;
ValidatorSetState storage vset;
if (commitment.validatorSetID == nextValidatorSet.id) {
if (commitment.validatorSetID == currentValidatorSet.id || commitment.validatorSetID == nextValidatorSet.id - 1)
{
vset = currentValidatorSet;
} else if (commitment.validatorSetID >= nextValidatorSet.id) {
is_next_session = true;
vset = nextValidatorSet;
} else if (commitment.validatorSetID == currentValidatorSet.id) {
vset = currentValidatorSet;
} else {
revert InvalidCommitment();
}
Expand All @@ -368,7 +370,7 @@ contract BeefyClient {
bytes32 newMMRRoot = ensureProvidesMMRRoot(commitment);

if (is_next_session) {
if (leaf.nextAuthoritySetID != nextValidatorSet.id + 1) {
if (leaf.nextAuthoritySetID <= nextValidatorSet.id) {
revert InvalidMMRLeaf();
}
bool leafIsValid =
Expand Down
115 changes: 111 additions & 4 deletions contracts/test/BeefyClient.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ contract BeefyClientTest is Test {
bitSetArray = beefyValidatorSetRaw.readUintArray(".participants");
absentBitSetArray = beefyValidatorSetRaw.readUintArray(".absentees");

console.log("current validator's merkle root is: %s", Strings.toHexString(uint256(root), 32));

beefyClient = new BeefyClientMock(randaoCommitDelay, randaoCommitExpiration, minNumRequiredSignatures);

bitfield = beefyClient.createInitialBitfield(bitSetArray, setSize);
Expand Down Expand Up @@ -101,6 +99,20 @@ contract BeefyClientTest is Test {
return BeefyClient.Commitment(blockNumber, setId, payload);
}

function initializeNonConsecutive(uint32 _setId, uint32 _nextSetId)
public
returns (BeefyClient.Commitment memory)
{
currentSetId = _setId;
nextSetId = _nextSetId;
BeefyClient.ValidatorSet memory vset = BeefyClient.ValidatorSet(currentSetId, setSize, root);
BeefyClient.ValidatorSet memory nextvset = BeefyClient.ValidatorSet(nextSetId, setSize, root);
beefyClient.initialize_public(0, vset, nextvset);
BeefyClient.PayloadItem[] memory payload = new BeefyClient.PayloadItem[](1);
payload[0] = BeefyClient.PayloadItem(mmrRootID, abi.encodePacked(mmrRoot));
return BeefyClient.Commitment(blockNumber, setId, payload);
}

function printBitArray(uint256[] memory bits) private view {
for (uint256 i = 0; i < bits.length; i++) {
console.log("bits index at %d is %d", i, bits[i]);
Expand Down Expand Up @@ -166,7 +178,7 @@ contract BeefyClientTest is Test {
);
}

function testSubmit() public returns (BeefyClient.Commitment memory) {
function testSubmitHappyPath() public returns (BeefyClient.Commitment memory) {
BeefyClient.Commitment memory commitment = initialize(setId);

assertEq(beefyClient.getValidatorCounter(false, finalValidatorProofs[0].index), 0);
Expand Down Expand Up @@ -388,7 +400,7 @@ contract BeefyClientTest is Test {
commitPrevRandao();
}

function testSubmitWithHandover() public {
function testSubmitWithHandoverHappyPath() public {
//initialize with previous set
BeefyClient.Commitment memory commitment = initialize(setId - 1);

Expand Down Expand Up @@ -752,4 +764,99 @@ contract BeefyClientTest is Test {
BeefyClient.ValidatorSet(nextId, 0, 0x0)
);
}

function testSubmitNonConsecutive() public {
BeefyClient.Commitment memory commitment = initializeNonConsecutive(setId, setId + 2);

beefyClient.submitInitial(commitment, bitfield, finalValidatorProofs[0]);

vm.roll(block.number + randaoCommitDelay);

commitPrevRandao();

createFinalProofs();

beefyClient.submitFinal(
commitment, bitfield, finalValidatorProofs, emptyLeaf, emptyLeafProofs, emptyLeafProofOrder
);

assertEq(beefyClient.latestBeefyBlock(), blockNumber);

(uint128 _currentSetId,,,) = beefyClient.currentValidatorSet();
assertEq(_currentSetId, uint128(setId));

(uint128 _nextSetId,,,) = beefyClient.nextValidatorSet();
assertEq(_nextSetId, uint128(setId + 2));
}

function testSubmitNonConsecutiveCommitNotInCurrentSet() public {
BeefyClient.Commitment memory commitment = initializeNonConsecutive(setId - 1, setId + 1);

beefyClient.submitInitial(commitment, bitfield, finalValidatorProofs[0]);

vm.roll(block.number + randaoCommitDelay);

commitPrevRandao();

createFinalProofs();

beefyClient.submitFinal(commitment, bitfield, finalValidatorProofs, mmrLeaf, mmrLeafProofs, leafProofOrder);
assertEq(beefyClient.latestBeefyBlock(), blockNumber);

(uint128 _currentSetId,,,) = beefyClient.currentValidatorSet();
assertEq(_currentSetId, uint128(setId - 1));

(uint128 _nextSetId,,,) = beefyClient.nextValidatorSet();
assertEq(_nextSetId, uint128(setId + 1));
}

function testSubmitWithHandoverNonConsecutive() public {
BeefyClient.Commitment memory commitment = initializeNonConsecutive(setId - 2, setId);

beefyClient.submitInitial(commitment, bitfield, finalValidatorProofs[0]);

vm.roll(block.number + randaoCommitDelay);

commitPrevRandao();

createFinalProofs();

beefyClient.submitFinal(commitment, bitfield, finalValidatorProofs, mmrLeaf, mmrLeafProofs, leafProofOrder);
assertEq(beefyClient.latestBeefyBlock(), blockNumber);

(uint128 _currentSetId,,,) = beefyClient.currentValidatorSet();
assertEq(_currentSetId, uint128(setId));

(uint128 _nextSetId,,,) = beefyClient.nextValidatorSet();
assertEq(_nextSetId, uint128(setId + 1));
}

function testSubmitWithHandoverNonConsecutiveCommitmentMoreThanNextSetID() public {
BeefyClient.Commitment memory commitment = initializeNonConsecutive(setId - 2, setId - 1);

beefyClient.submitInitial(commitment, bitfield, finalValidatorProofs[0]);

vm.roll(block.number + randaoCommitDelay);

commitPrevRandao();

createFinalProofs();

beefyClient.submitFinal(commitment, bitfield, finalValidatorProofs, mmrLeaf, mmrLeafProofs, leafProofOrder);
assertEq(beefyClient.latestBeefyBlock(), blockNumber);

(uint128 _currentSetId,,,) = beefyClient.currentValidatorSet();
assertEq(_currentSetId, uint128(setId - 1));

(uint128 _nextSetId,,,) = beefyClient.nextValidatorSet();
assertEq(_nextSetId, uint128(setId + 1));
}

function testSubmitNonConsecutiveCommitInvalidSetId() public {
BeefyClient.Commitment memory commitment = initializeNonConsecutive(setId - 1, setId + 2);

vm.expectRevert(BeefyClient.InvalidCommitment.selector);

beefyClient.submitInitial(commitment, bitfield, finalValidatorProofs[0]);
}
}
1 change: 0 additions & 1 deletion contracts/test/mocks/BeefyClientMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ contract BeefyClientMock is BeefyClient {
nextValidatorSet.length = _nextValidatorSet.length;
nextValidatorSet.root = _nextValidatorSet.root;
nextValidatorSet.usageCounters = createUint16Array(nextValidatorSet.length);
console.log(currentValidatorSet.usageCounters.data.length);
}

// Used to verify integrity of storage to storage copies
Expand Down
15 changes: 15 additions & 0 deletions relayer/chain/relaychain/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package relaychain
import (
"context"
"fmt"
"strings"

gsrpc "github.com/snowfork/go-substrate-rpc-client/v4"
"github.com/snowfork/go-substrate-rpc-client/v4/types"
Expand All @@ -18,6 +19,7 @@ type Connection struct {
api *gsrpc.SubstrateAPI
metadata types.Metadata
genesisHash types.Hash
isRococo bool
}

func NewConnection(endpoint string) *Connection {
Expand All @@ -34,6 +36,10 @@ func (co *Connection) Metadata() *types.Metadata {
return &co.metadata
}

func (co *Connection) IsRococo() bool {
return co.isRococo
}

func (co *Connection) Connect(_ context.Context) error {
// Initialize API
api, err := gsrpc.NewSubstrateAPI(co.endpoint)
Expand All @@ -56,6 +62,15 @@ func (co *Connection) Connect(_ context.Context) error {
}
co.genesisHash = genesisHash

// Fetch chain name
chainName, err := api.RPC.System.Chain()
if err != nil {
return err
}
if strings.HasPrefix(string(chainName), "Rococo") {
co.isRococo = true
}

log.WithFields(log.Fields{
"endpoint": co.endpoint,
"metaVersion": meta.Version,
Expand Down
6 changes: 5 additions & 1 deletion relayer/cmd/scan_beefy.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ func ScanBeefyFn(cmd *cobra.Command, _ []string) error {
"validator-set-id": validatorSetID,
}).Info("Connected to relaychain.")

commitments, err := polkadotListener.Start(ctx, eg, beefyBlock, validatorSetID)
var currentState beefy.BeefyClientState
currentState.CurrentValidatorSetID = validatorSetID
currentState.LatestBeefyBlock = beefyBlock

commitments, err := polkadotListener.Start(ctx, eg, &currentState)
if err != nil {
logrus.WithError(err).Fatalf("could not start")
}
Expand Down
10 changes: 9 additions & 1 deletion relayer/relays/beefy/ethereum-writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ func (wr *EthereumWriter) Start(ctx context.Context, eg *errgroup.Group, request
// Mandatory commitments are always signed by the next validator set recorded in
// the beefy light client
task.ValidatorsRoot = state.NextValidatorSetRoot
if task.ValidatorsRoot == task.NextValidatorsRoot {
log.WithFields(logrus.Fields{
"beefyBlockNumber": task.SignedCommitment.Commitment.BlockNumber,
"latestBeefyBlock": state.LatestBeefyBlock,
"validatorSetId": task.SignedCommitment.Commitment.ValidatorSetID,
"nextValidatorSetId": state.NextValidatorSetID,
}).Info("beefy authorities not changed")
continue
}

err = wr.submit(ctx, task)
if err != nil {
Expand Down Expand Up @@ -96,7 +105,6 @@ func (wr *EthereumWriter) queryBeefyClientState(ctx context.Context) (*BeefyClie
if err != nil {
return nil, err
}

currentValidatorSet, err := wr.contract.CurrentValidatorSet(&callOpts)
if err != nil {
return nil, err
Expand Down
20 changes: 11 additions & 9 deletions relayer/relays/beefy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (relay *Relay) Start(ctx context.Context, eg *errgroup.Group) error {
"validatorSetID": initialState.CurrentValidatorSetID,
}).Info("Retrieved current BeefyClient state")

requests, err := relay.polkadotListener.Start(ctx, eg, initialState.LatestBeefyBlock, initialState.CurrentValidatorSetID)
requests, err := relay.polkadotListener.Start(ctx, eg, initialState)
if err != nil {
return fmt.Errorf("initialize polkadot listener: %w", err)
}
Expand Down Expand Up @@ -128,17 +128,19 @@ func (relay *Relay) OneShotSync(ctx context.Context, blockNumber uint64) error {
return nil
}
if task.SignedCommitment.Commitment.ValidatorSetID > state.NextValidatorSetID {
log.WithFields(log.Fields{
"latestBeefyBlock": state.LatestBeefyBlock,
"currentValidatorSetID": state.CurrentValidatorSetID,
"nextValidatorSetID": state.NextValidatorSetID,
"validatorSetIDToSync": task.SignedCommitment.Commitment.ValidatorSetID,
}).Warn("Task unexpected, wait for mandatory updates to catch up first")
return nil
if task.NextValidatorsRoot != state.NextValidatorSetRoot {
log.WithFields(log.Fields{
"latestBeefyBlock": state.LatestBeefyBlock,
"currentValidatorSetID": state.CurrentValidatorSetID,
"nextValidatorSetID": state.NextValidatorSetID,
"validatorSetIDToSync": task.SignedCommitment.Commitment.ValidatorSetID,
}).Warn("Task unexpected, wait for mandatory updates to catch up first")
return nil
}
}

// Submit the task
if task.SignedCommitment.Commitment.ValidatorSetID == state.CurrentValidatorSetID {
if task.SignedCommitment.Commitment.ValidatorSetID == state.CurrentValidatorSetID || task.SignedCommitment.Commitment.ValidatorSetID == state.NextValidatorSetID-1 {
task.ValidatorsRoot = state.CurrentValidatorSetRoot
} else {
task.ValidatorsRoot = state.NextValidatorSetRoot
Expand Down
Loading
Loading