Skip to content

remove dependency of validator.State from BitSetSignature.Verify #3679

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

Merged
merged 16 commits into from
Jan 29, 2025

Conversation

tsachiherman
Copy link
Contributor

@tsachiherman tsachiherman commented Jan 27, 2025

Why this should be merged

This PR minimize the interface scope of the pChainState interface argument in BitSetSignature.Verify.

How this works

Existing interface of BitSetSignature.Verify is

func (s *BitSetSignature) Verify(
	ctx context.Context,
	msg *UnsignedMessage,
	networkID uint32,
	pChainState validators.State,
	pChainHeight uint64,
	quorumNum uint64,
	quorumDen uint64,
)

This PR replaces the interface with

func (s *BitSetSignature) Verify(
	msg *UnsignedMessage,
	networkID uint32,
	validators CanonicalValidatorSet,
	quorumNum uint64,
	quorumDen uint64,
) error

with the CanonicalValidatorSet defined as

type CanonicalValidatorSet struct {
	Validators  []*Validator
	TotalWeight uint64
}

The CanonicalValidatorSet construction function was added as well :

func GetCanonicalValidatorSetFromState(
        ctx context.Context,
	pChainState validators.State,
	pChainHeight uint64,
	sourceChainID ids.ID,
) (CanonicalValidatorSet, error) 

How this was tested

Existing testing were modified.

Need to be documented in RELEASES.md?

No.

Corresponding changes

coreth : ava-labs/coreth#769

@StephenButtolph
Copy link
Contributor

StephenButtolph commented Jan 27, 2025

The comment:

// Because [signers] is a subset of [vdrs], this can never error.
sigWeight, _ := SumWeight(signers)

is no longer valid right?

@tsachiherman tsachiherman marked this pull request as ready for review January 28, 2025 17:31
@tsachiherman tsachiherman self-assigned this Jan 28, 2025
@StephenButtolph StephenButtolph added this pull request to the merge queue Jan 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 29, 2025
@tsachiherman tsachiherman added this pull request to the merge queue Jan 29, 2025
Merged via the queue into master with commit 6485404 Jan 29, 2025
22 checks passed
@tsachiherman tsachiherman deleted the tsachi/minimize-verify-interface2 branch January 29, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants