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.

Sign up for GitHub

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

Jump to bottom

Consistently use cert pointer in verification #751

Merged
merged 1 commit into from
Nov 21, 2024
+16 −16
Merged

Consistently use cert pointer in verification #751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion certexchange/polling/poller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (p *Poller) Poll(ctx context.Context, peer peer.ID) (*PollResult, error) {
// TODO: consider batching verification, it's slightly faster.
next, _, pt, err := certs.ValidateFinalityCertificates(
p.SignatureVerifier, p.NetworkName, p.PowerTable, p.NextInstance, nil,
*cert,
cert,
)
if err != nil {
res.Status = PollIllegal
Expand Down
4 changes: 2 additions & 2 deletions certs/certs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func NewFinalityCertificate(powerDelta PowerTableDiff, justification *gpbft.Just
// finalized, the instance of the first invalid finality certificate, and the power table that
// should be used to validate that finality certificate, along with the error encountered.
func ValidateFinalityCertificates(verifier gpbft.Verifier, network gpbft.NetworkName, prevPowerTable gpbft.PowerEntries, nextInstance uint64, base *gpbft.TipSet,
certs ...FinalityCertificate) (_nextInstance uint64, chain gpbft.ECChain, newPowerTable gpbft.PowerEntries, err error) {
certs ...*FinalityCertificate) (_nextInstance uint64, chain gpbft.ECChain, newPowerTable gpbft.PowerEntries, err error) {
for _, cert := range certs {
if cert.GPBFTInstance != nextInstance {
return nextInstance, chain, prevPowerTable, fmt.Errorf("expected instance %d, found instance %d", nextInstance, cert.GPBFTInstance)
Expand Down Expand Up @@ -144,7 +144,7 @@ func ValidateFinalityCertificates(verifier gpbft.Verifier, network gpbft.Network
// Verify the signature of the given finality certificate. This doesn't validate the power delta, or
// any other parts of the certificate, just that the _value_ has been signed by a majority of the
// power.
func verifyFinalityCertificateSignature(verifier gpbft.Verifier, powerTable gpbft.PowerEntries, nn gpbft.NetworkName, cert FinalityCertificate) error {
func verifyFinalityCertificateSignature(verifier gpbft.Verifier, powerTable gpbft.PowerEntries, nn gpbft.NetworkName, cert *FinalityCertificate) error {
scaled, totalScaled, err := powerTable.Scaled()
if err != nil {
return fmt.Errorf("failed to scale power table: %w", err)
Expand Down
24 changes: 12 additions & 12 deletions certs/certs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func TestFinalityCertificates(t *testing.T) {
tsg := sim.NewTipSetGenerator(rng.Uint64())
base := gpbft.TipSet{Epoch: 0, Key: tsg.Sample(), PowerTable: tableCid}

certificates := make([]certs.FinalityCertificate, 10)
certificates := make([]*certs.FinalityCertificate, 10)
powerTables := make([]gpbft.PowerEntries, 10)
var removedPowerEntries []gpbft.PowerEntry
for i := range certificates {
Expand All @@ -223,7 +223,7 @@ func TestFinalityCertificates(t *testing.T) {
justification := makeJustification(t, rng, tsg, backend, base, uint64(i), powerTables[i], powerTable)
cert, err := certs.NewFinalityCertificate(certs.MakePowerTableDiff(powerTables[i], powerTable), justification)
require.NoError(t, err)
certificates[i] = *cert
certificates[i] = cert
base = *justification.Vote.Value.Head()
}

Expand Down Expand Up @@ -291,15 +291,15 @@ func TestBadFinalityCertificates(t *testing.T) {

// Unexpected instance number
{
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 0, nil, *certificate)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 0, nil, certificate)
require.ErrorContains(t, err, "expected instance 0, found instance 1")
require.EqualValues(t, 0, nextInstance)
require.Equal(t, powerTable, newPowerTable)
require.Empty(t, chain)
}
// Wrong base.
{
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, certificate.ECChain.Head(), *certificate)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, certificate.ECChain.Head(), certificate)
require.ErrorContains(t, err, "base tipset does not match finalized chain")
require.EqualValues(t, 1, nextInstance)
require.Equal(t, powerTable, newPowerTable)
Expand All @@ -309,7 +309,7 @@ func TestBadFinalityCertificates(t *testing.T) {
// Discard most of the power table. Given the initial power distribution, we can guarantee
// that we require more than 10 participants.
{
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable[:10], 1, nil, *certificate)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable[:10], 1, nil, certificate)
require.ErrorContains(t, err, "but we only have 10 entries in the power table")
require.EqualValues(t, 1, nextInstance)
require.Equal(t, powerTable[:10], newPowerTable)
Expand All @@ -322,7 +322,7 @@ func TestBadFinalityCertificates(t *testing.T) {
require.NoError(t, err)
powerTableCpy := slices.Clone(powerTable)
powerTableCpy[firstSigner].PubKey = powerTableCpy[(int(firstSigner)+1)%len(powerTableCpy)].PubKey
nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, *certificate)
nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, certificate)
require.ErrorContains(t, err, "invalid signature on finality certificate")
require.EqualValues(t, 1, nextInstance)
require.Empty(t, chain)
Expand All @@ -335,7 +335,7 @@ func TestBadFinalityCertificates(t *testing.T) {
powerTableCpy := slices.Clone(powerTable)
// increase so we definitely have enough power
powerTableCpy[firstSigner].Power = big.Add(powerTableCpy[firstSigner].Power, gpbft.NewStoragePower(1))
nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, *certificate)
nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, certificate)
require.ErrorContains(t, err, "incorrect power diff")
require.EqualValues(t, 1, nextInstance)
require.Empty(t, chain)
Expand All @@ -352,7 +352,7 @@ func TestBadFinalityCertificates(t *testing.T) {
require.NoError(t, err)
powerTableCpy[firstSigner].Power = gpbft.NewStoragePower(0xffff)

nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, *certificate)
nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, certificate)
require.ErrorContains(t, err, "no effective power after scaling")
require.EqualValues(t, 1, nextInstance)
require.Empty(t, chain)
Expand All @@ -373,7 +373,7 @@ func TestBadFinalityCertificates(t *testing.T) {
return nil
}))

nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, *certificate)
nextInstance, chain, _, err := certs.ValidateFinalityCertificates(backend, networkName, powerTableCpy, 1, nil, certificate)
require.ErrorContains(t, err, fmt.Sprintf("has insufficient power: %d < 2/3 %d", activePower, totalPower))
require.EqualValues(t, 1, nextInstance)
require.Empty(t, chain)
Expand All @@ -383,7 +383,7 @@ func TestBadFinalityCertificates(t *testing.T) {
{
certCpy := *certificate
certCpy.ECChain = nil
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, nil, certCpy)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, nil, &certCpy)
require.ErrorContains(t, err, "empty finality certificate")
require.EqualValues(t, 1, nextInstance)
require.Equal(t, powerTable, newPowerTable)
Expand All @@ -395,7 +395,7 @@ func TestBadFinalityCertificates(t *testing.T) {
certCpy := *certificate
certCpy.ECChain = slices.Clone(certCpy.ECChain)
slices.Reverse(certCpy.ECChain)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, nil, certCpy)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, nil, &certCpy)
require.ErrorContains(t, err, "chain must have increasing epochs")
require.EqualValues(t, 1, nextInstance)
require.Equal(t, powerTable, newPowerTable)
Expand All @@ -411,7 +411,7 @@ func TestBadFinalityCertificates(t *testing.T) {
certCpy.PowerTableDelta[0].PowerDelta = gpbft.NewStoragePower(0)
certCpy.PowerTableDelta[0].SigningKey = nil

nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, nil, certCpy)
nextInstance, chain, newPowerTable, err := certs.ValidateFinalityCertificates(backend, networkName, powerTable, 1, nil, &certCpy)
require.ErrorContains(t, err, "failed to apply power table delta")
require.EqualValues(t, 1, nextInstance)
require.Equal(t, powerTable, newPowerTable)
Expand Down
2 changes: 1 addition & 1 deletion host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ func (h *gpbftHost) saveDecision(decision *gpbft.Justification) (*certs.Finality
if err != nil {
return nil, fmt.Errorf("forming certificate out of decision: %w", err)
}
_, _, _, err = certs.ValidateFinalityCertificates(h, h.NetworkName(), current.PowerTable.Entries, decision.Vote.Instance, nil, *cert)
_, _, _, err = certs.ValidateFinalityCertificates(h, h.NetworkName(), current.PowerTable.Entries, decision.Vote.Instance, nil, cert)
if err != nil {
return nil, fmt.Errorf("certificate is invalid: %w", err)
}
Expand Down
Loading