Skip to content

Commit

Permalink
fix(f3): poll the lease by repeatedly participating instead of checki…
Browse files Browse the repository at this point in the history
…ng progress
  • Loading branch information
simlecode committed Oct 31, 2024
1 parent 1cf79da commit 9effc9d
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 29 deletions.
24 changes: 18 additions & 6 deletions pkg/vf3/participation_lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type f3Status = func() (*manifest.Manifest, gpbft.Instant)
type leaser struct {
mutex sync.Mutex
leases map[uint64]types.F3ParticipationLease
issuer peer.ID
issuer string // issuer is the base58 encoding of the node peer ID.
status f3Status
maxLeasableInstances uint64
// Signals that a lease was created and/or updated.
Expand All @@ -29,15 +29,17 @@ type leaser struct {
func newParticipationLeaser(nodeID peer.ID, status f3Status, maxLeasedInstances uint64) *leaser {
return &leaser{
leases: make(map[uint64]types.F3ParticipationLease),
issuer: nodeID,
issuer: nodeID.String(),
status: status,
maxLeasableInstances: maxLeasedInstances,
notifyParticipation: make(chan struct{}, 1),
}
}

func (l *leaser) getOrRenewParticipationTicket(participant uint64, previous types.F3ParticipationTicket, instances uint64) (types.F3ParticipationTicket, error) {

if instances == 0 {
return nil, errors.New("not enough instances")
}
if instances > l.maxLeasableInstances {
return nil, types.ErrF3ParticipationTooManyInstances
}
Expand Down Expand Up @@ -96,15 +98,25 @@ func (l *leaser) participate(ticket types.F3ParticipationTicket) (types.F3Partic
l.mutex.Lock()
defer l.mutex.Unlock()
currentLease, found := l.leases[newLease.MinerID]
if found && currentLease.Network == newLease.Network && currentLease.FromInstance > newLease.FromInstance {
// For safety, strictly require lease start instance to never decrease.
return types.F3ParticipationLease{}, types.ErrF3ParticipationTicketStartBeforeExisting
if found {
// short-circuite for reparticipation.
if currentLease == newLease {
return newLease, nil
}
if currentLease.Network == newLease.Network && currentLease.FromInstance > newLease.FromInstance {
// For safety, strictly require lease start instance to never decrease.
return types.F3ParticipationLease{}, types.ErrF3ParticipationTicketStartBeforeExisting
}
} else {
log.Infof("started participating in F3 for miner %d", newLease.MinerID)
}
l.leases[newLease.MinerID] = newLease
select {
case l.notifyParticipation <- struct{}{}:
default:
}
newLease.ValidityTerm = newLease.ToInstance() - instant.ID
newLease.FromInstance = instant.ID
return newLease, nil
}

Expand Down
44 changes: 30 additions & 14 deletions pkg/vf3/participation_lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,21 @@ import (

"github.com/filecoin-project/go-f3/gpbft"
"github.com/filecoin-project/go-f3/manifest"
tf "github.com/filecoin-project/venus/pkg/testhelpers/testflags"
"github.com/filecoin-project/venus/venus-shared/types"
)

func TestLeaser(t *testing.T) {
nodeID := peer.ID("peerID")
tf.UnitTest(t)
issuer := peer.ID("peerID")
progress := mockProgress{currentInstance: 10}
subject := newParticipationLeaser(nodeID, progress.Progress, 5)

t.Run("participate", func(t *testing.T) {
ticket, err := subject.getOrRenewParticipationTicket(123, nil, 5)
require.NoError(t, err)

lease, err := subject.participate(ticket)
require.NoError(t, err)
require.Equal(t, uint64(123), lease.MinerID)
require.Equal(t, nodeID, lease.Issuer)
require.Equal(t, uint64(5), lease.ValidityTerm) // Current instance (10) + offset (5)
subject := newParticipationLeaser(issuer, progress.Progress, 5)
t.Run("participate zero", func(t *testing.T) {
ticket, err := subject.getOrRenewParticipationTicket(123, nil, 0)
require.Error(t, err)
require.Nil(t, ticket)
})

t.Run("get participants", func(t *testing.T) {
progress.currentInstance = 11
ticket1, err := subject.getOrRenewParticipationTicket(123, nil, 4)
Expand All @@ -46,6 +43,23 @@ func TestLeaser(t *testing.T) {
require.Contains(t, participants, uint64(123))
require.Contains(t, participants, uint64(456))

leases := subject.getValidLeases()
require.Len(t, leases, 2)
require.Contains(t, leases, types.F3ParticipationLease{
Network: testManifest.NetworkName,
Issuer: issuer.String(),
MinerID: 123,
FromInstance: 11,
ValidityTerm: 4,
})
require.Contains(t, leases, types.F3ParticipationLease{
Network: testManifest.NetworkName,
Issuer: issuer.String(),
MinerID: 456,
FromInstance: 11,
ValidityTerm: 5,
})

// After instance 16, only participant 456 should be valid.
participants = subject.getParticipantsByInstance(testManifest.NetworkName, 16)
require.Len(t, participants, 1)
Expand All @@ -71,7 +85,7 @@ func TestLeaser(t *testing.T) {

// Generate a token from the same subject but with higher term, then assert that
// original subject with lower term rejects it.
subjectSpoofWithHigherMaxLease := newParticipationLeaser(nodeID, progress.Progress, 6)
subjectSpoofWithHigherMaxLease := newParticipationLeaser(issuer, progress.Progress, 6)
ticket, err = subjectSpoofWithHigherMaxLease.getOrRenewParticipationTicket(123, nil, 6)
require.NoError(t, err)
require.NotEmpty(t, ticket)
Expand Down Expand Up @@ -138,6 +152,7 @@ func TestLeaser(t *testing.T) {
// Get or renew with valid but mismatching issuer
progress.currentInstance -= 10
anotherIssuer := newParticipationLeaser("barreleye", progress.Progress, 5)
require.NoError(t, err)
newTicket, err = anotherIssuer.getOrRenewParticipationTicket(123, previous, 5)
require.ErrorIs(t, err, types.ErrF3ParticipationIssuerMismatch)
require.Empty(t, newTicket)
Expand All @@ -151,7 +166,8 @@ func TestLeaser(t *testing.T) {

// Get or renew with expired but mismatching session
progress.currentInstance -= 10
subjectAtNewSession := newParticipationLeaser(nodeID, progress.Progress, 5)
subjectAtNewSession := newParticipationLeaser(issuer, progress.Progress, 5)
require.NoError(t, err)
newTicket, err = subjectAtNewSession.getOrRenewParticipationTicket(123, previous, 5)
require.NoError(t, err)
require.NotNil(t, newTicket)
Expand Down
10 changes: 10 additions & 0 deletions venus-devtool/api-gen/example.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,16 @@ func init() {
}
addExample(&ethTraceFilterCriteria)
addExample(ethTraceFilterCriteria)

f3Lease := types.F3ParticipationLease{
Network: "filecoin",
Issuer: pid.String(),
MinerID: 1234,
FromInstance: 10,
ValidityTerm: 15,
}
addExample(f3Lease)
addExample(&f3Lease)
}

func ExampleValue(method string, t, parent reflect.Type) interface{} {
Expand Down
6 changes: 3 additions & 3 deletions venus-shared/api/chain/v1/method.md
Original file line number Diff line number Diff line change
Expand Up @@ -3859,9 +3859,9 @@ Response:
{
"Network": "filecoin",
"Issuer": "12D3KooWGzxzKZYveHXtpG6AsrUJBcWxHBFS2HsEoGTxrMLvKXtf",
"MinerID": 42,
"FromInstance": 42,
"ValidityTerm": 42
"MinerID": 1234,
"FromInstance": 10,
"ValidityTerm": 15
}
```

Expand Down
4 changes: 2 additions & 2 deletions venus-shared/types/api_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,8 @@ type F3ParticipationTicket []byte
type F3ParticipationLease struct {
// Network is the name of the network this lease belongs to.
Network gpbft.NetworkName
// Issuer is the identity of the node that issued the lease.
Issuer peer.ID
// Issuer is the identity of the node that issued the lease, encoded as base58.
Issuer string
// MinerID is the actor ID of the miner that holds the lease.
MinerID uint64
// FromInstance specifies the instance ID from which this lease is valid.
Expand Down
7 changes: 3 additions & 4 deletions venus-shared/types/cbor_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 9effc9d

Please sign in to comment.