maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic #2151

noot · 2022-01-07T22:44:21Z

Changes

refactor babe epoch handling logic to be separated into an epochHandler module, which will handle the authorship slots for that epoch

Tests

go test ./lib/babe

Issues

closes devnet: BABE does not restart after pausing, and pauses at wrong times #2097

Primary Reviewer

@EclesioMeloJunior

…/babe-refactor

noot · 2022-01-19T22:11:27Z

dot/peerset/peerset.go

@@ -340,8 +342,9 @@ func (ps *PeerSet) reportPeer(change ReputationChange, peers ...peer.ID) error {
 					return err
 				}

+				// TODO: change Noop to Drop once #2098 is fixed


let me know if i should put commenting out this ban peer stuff into another PR

Can you not just uncomment this out for now? I wasn't able to reproduce the nodes banning at this point at least locally.

noot · 2022-01-19T22:13:08Z

lib/babe/epoch_test.go

+	"github.com/stretchr/testify/require"
+)
+
+var keyring, _ = keystore.NewSr25519Keyring() //nolint:typecheck


when I don't declare keyring here I get an undeclared error, but I then get a lint error since it's defined in babe_integration_test.go. how do i fix this?

nvm, i just gotta put it in a non integration test file

lib/babe/babe.go

lib/babe/build.go

lib/babe/build_integration_test.go

lib/babe/babe_integration_test.go

qdm12 · 2022-01-21T19:22:07Z

lib/babe/epoch_handler_test.go

+	require.Equal(t, 200, len(eh.slotToProof))
+	require.Equal(t, uint64(1), eh.epochNumber)
+	require.Equal(t, getSlotStartTime(9999, sd), eh.startTime)
+	require.Equal(t, uint64(9999), eh.firstSlot)
+	require.Equal(t, constants, eh.constants)
+	require.Equal(t, epochData, eh.epochData)
+	require.NotNil(t, eh.handleSlot)


Can't you assert the entire eh? Also eh is eh variable naming wise 😄

I can't assert the entire epochHandler because the slotToProof value is uses randomness in the VRF calculation

Can't we assign it some predictable value in the test? Or perhaps dependency inject the source of randomness? 🤔

hmm it would probably require overriding the claimPrimarySlot function which is used to generate the proof... I can do it if you'd really like but seems like that sorta veers into territory of changing the code for tests

As in if it's a lot of code change, don't; otherwise I'd say yes your code should be changed to be more testable 😉

lib/babe/epoch_handler.go

timwu20 · 2022-01-22T02:07:07Z

lib/babe/babe.go

-			return err
-		}
+		err = func() error {
+			ctx, cancel := context.WithCancel(b.ctx)


I'm of the proponent where we should not be using context.Context within package boundaries. I know we already do this in most of the Service implementations. If what we're looking for is cancellation, then we can just create a channel for that purpose. context.Context is meant to traverse through API boundaries and contain request scoped data. I've seen a lot of examples of go code using them to handle long running processes and I feel like it introduces more weight than required.

In this case the Service.ctx is being wrapped to add cancellation to the current epochHandler. So to my knowledge that the Service.ctx was created with context.WithCancel, then ctx.Done() would close if the original CancelFunc was called. But we wrap this context again in line 347. So does that mean that if we call the local cancel CancelFunc than any goroutines listening on Service.ctx would receive the channel close? I would assume no, but these are things that make the code more difficult to understand. I think as a best practice you shouldn't call context.WithCancel or any of the other context.WithXXX functions on supplied contexts.

I think epochHandler.run() should be modified to accept a context.Context that's not the original one stored in Service. This runEngine function should listen on the Service one and trigger cancellation within this function for the current epochHandler.

noot · 2022-01-24T18:19:34Z

@timwu20 made your suggested change regarding the context, let me know what you think now!

lib/babe/babe.go

lib/babe/epoch_handler.go

EclesioMeloJunior

Just small changes!

qdm12 · 2022-01-24T22:17:21Z

lib/babe/babe.go

-
-	if err = babeService.setupParameters(cfg); err != nil {
-		return nil, err
+		constants: &constants{


Why is a pointer by the way?

i don't know lol. want me to change it?

Yes pleaaaase

qdm12 · 2022-01-24T22:18:44Z

lib/babe/babe.go

+	auths := make([]types.Authority, len(b.epochHandler.epochData.authorities))
+	copy(auths, b.epochHandler.epochData.authorities)
+	return auths


I don't think you can copy types.Authority directly, you would need a deep copy method I guess?

qdm12 · 2022-01-24T22:19:35Z

lib/babe/babe.go

+	// retry to run the engine at some point (maybe the next epoch) if
+	// there's an error.
+	if err := b.runEngine(); err != nil {
+		logger.Criticalf("failed to run block production engine: %s", err)


lib/babe/babe.go

qdm12 · 2022-01-24T22:20:39Z

lib/babe/babe.go

 	if err != nil {
-		logger.Errorf("failed to get current epoch: %s", err)
-		return err
+		return nil, fmt.Errorf("failed to initiate epoch %d: %s", epoch, err)


Suggested change

return nil, fmt.Errorf("failed to initiate epoch %d: %s", epoch, err)

return nil, fmt.Errorf("failed to initiate epoch %d: %w", epoch, err)

done (also fixed all the other error returns)

qdm12 · 2022-01-24T22:20:53Z

lib/babe/babe.go

-		}
+	epochStartSlot, err := b.epochState.GetStartSlotForEpoch(epoch)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get start slot for current epoch %d: %s", epoch, err)


Suggested change

return nil, fmt.Errorf("failed to get start slot for current epoch %d: %s", epoch, err)

return nil, fmt.Errorf("failed to get start slot for current epoch %d: %w", epoch, err)

qdm12 · 2022-01-24T22:21:13Z

lib/babe/babe.go

+func (b *Service) runEngine() error {
+	epoch, err := b.epochState.GetCurrentEpoch()
+	if err != nil {
+		return fmt.Errorf("failed to get current epoch: %s", err)


Suggested change

return fmt.Errorf("failed to get current epoch: %s", err)

return fmt.Errorf("failed to get current epoch: %w", err)

qdm12 · 2022-01-24T22:21:36Z

lib/babe/babe.go

-			return err
-		}
+	for {
+		err = func() error {


can we have this one as a separate global scope function?

qdm12 · 2022-01-24T22:21:56Z

lib/babe/babe.go

+			defer cancel()
+			b.epochHandler, err = b.initiateAndGetEpochHandler(epoch)
+			if err != nil {
+				return err


Suggested change

return err

return fmt.Errorf("cannot initiate epoch handler: %w", err)

qdm12 · 2022-01-24T22:22:10Z

lib/babe/babe.go

+			// get start slot for current epoch
+			nextEpochStart, err := b.epochState.GetStartSlotForEpoch(epoch + 1)
+			if err != nil {
+				return fmt.Errorf("failed to get start slot for next epoch %d: %s", epoch+1, err)


Suggested change

return fmt.Errorf("failed to get start slot for next epoch %d: %s", epoch+1, err)

return fmt.Errorf("failed to get start slot for next epoch %d: %w", epoch+1, err)

qdm12 · 2022-01-24T22:23:26Z

lib/grandpa/grandpa.go

+			// TODO: sometimes grandpa fails to initiate due to a "Key not found"
+			// error, this shouldn't happen.
+			if err := s.initiate(); err != nil {
+				logger.Criticalf("failed to initiate: %s", err)


Panic? Error channel? 🤔

I think this is the same sort of issue as with BABE, we probably need to add an error channel so the higher-level service handler can either panic or restart the service. I'll make an issue for it

qdm12 · 2022-01-24T22:23:56Z

lib/babe/verify_integration_test.go

+	epochData.authorities = make([]types.Authority, 1)
+	epochData.authorities[0] = types.Authority{


Suggested change

epochData.authorities = make([]types.Authority, 1)

epochData.authorities[0] = types.Authority{

epochData.authorities = []types.Authority{

qdm12 · 2022-01-24T22:24:25Z

lib/babe/verify_integration_test.go

+	epochData.threshold = maxThreshold
+	epochData.authorityIndex = 0
+
+	var slotNumber uint64 = 1


Suggested change

var slotNumber uint64 = 1

const slotNumber = 1

qdm12 · 2022-01-24T22:24:45Z

lib/babe/verify_integration_test.go

@@ -214,14 +230,18 @@ func TestVerificationManager_VerifyBlock_Secondary(t *testing.T) {
 }

 func TestVerificationManager_VerifyBlock_MultipleEpochs(t *testing.T) {
+	t.Skip() // no idea why it's complaining it can't find the epoch data. fix later


qdm12 · 2022-01-24T22:25:15Z

lib/babe/verify_integration_test.go

+
+	if genCfg == nil {
+		genCfg = genesisBABEConfig
+	}
+


why is this moved?

if this isn't here the verifier tests panic :p

qdm12 · 2022-01-24T22:30:05Z

lib/babe/epoch_test.go

+	mockBlockState := NewMockBlockState(ctrl)
+	mockEpochState0 := NewMockEpochState(ctrl)
+	mockEpochState1 := NewMockEpochState(ctrl)
+	mockEpochState2 := NewMockEpochState(ctrl)
+
+	mockEpochState0.EXPECT().GetStartSlotForEpoch(gomock.Eq(uint64(0))).Return(uint64(1), nil)
+	mockEpochState1.EXPECT().GetStartSlotForEpoch(gomock.Eq(uint64(1))).Return(uint64(201), nil)
+	mockEpochState2.EXPECT().GetStartSlotForEpoch(gomock.Eq(uint64(1))).Return(uint64(201), nil)
+
+	mockEpochState1.EXPECT().HasEpochData(gomock.Eq(uint64(1))).Return(true, nil)
+	mockEpochState2.EXPECT().HasEpochData(gomock.Eq(uint64(1))).Return(true, nil)
+
+	kp := keyring.Alice().(*sr25519.Keypair)
+	authority := types.NewAuthority(kp.Public(), uint64(1))
+	testEpochData := &types.EpochData{
+		Randomness:  [32]byte{1},
+		Authorities: []types.Authority{*authority},
+	}
+
+	mockEpochState1.EXPECT().GetEpochData(gomock.Eq(uint64(1))).Return(testEpochData, nil)
+	mockEpochState2.EXPECT().GetEpochData(gomock.Eq(uint64(1))).Return(testEpochData, nil)
+
+	mockEpochState1.EXPECT().HasConfigData(gomock.Eq(uint64(1))).Return(true, nil)
+	mockEpochState2.EXPECT().HasConfigData(gomock.Eq(uint64(1))).Return(false, nil)
+
+	testConfigData := &types.ConfigData{
+		C1: 1,
+		C2: 1,
+	}
+
+	mockEpochState1.EXPECT().GetConfigData(gomock.Eq(uint64(1))).Return(testConfigData, nil)
+
+	testLatestConfigData := &types.ConfigData{
+		C1: 1,
+		C2: 2,
+	}
+
+	mockEpochState2.EXPECT().GetLatestConfigData().Return(testLatestConfigData, nil)
+
+	testEpochDataEpoch0 := &types.EpochData{
+		Randomness:  [32]byte{9},
+		Authorities: []types.Authority{*authority},
+	}
+
+	mockEpochState0.EXPECT().GetLatestEpochData().Return(testEpochDataEpoch0, nil)
+	mockEpochState0.EXPECT().GetLatestConfigData().Return(testConfigData, nil)


Perhaps you could configure your mocks according to test cases?

Like this for example.

There is also a reason where I believe having NewController(t) with the parent test might give some funny logs/behavior especially when running parallel subtests inside it.

yeah that would be nice, I'll work on that. gotta work on making my tests cleaner haha

lib/babe/babe.go

qdm12 · 2022-01-25T19:49:52Z

lib/babe/babe.go

-			defer timer.Stop()
-			select {
-			case <-timer.C:
+			if errors.Is(err, errServicePaused) || errors.Is(err, context.Canceled) {


Don't we want to return an error if the context gets canceled 🤔

we can? idk, the only time the context would be cancelled is if the node shutdowns afaik, so i don't think it matters to return an error

qdm12 · 2022-01-25T19:51:35Z

lib/babe/build_integration_test.go

+	babeService.epochHandler, err = babeService.initiateAndGetEpochHandler(0)
+	require.NoError(t, err)
+
+	authoringSlots := getAuthoringSlots(babeService.epochHandler.slotToProof)


nit just in case

Suggested change

authoringSlots := getAuthoringSlots(babeService.epochHandler.slotToProof)

authoringSlots := getAuthoringSlots(babeService.epochHandler.slotToProof)

require.NotEmpty(t, authoringSlots)

qdm12 · 2022-01-25T19:52:58Z

lib/babe/build_integration_test.go

 	}

 	rt, err := babeService.blockState.GetRuntime(nil)
 	require.NoError(t, err)

-	_, err = babeService.buildBlock(parentHeader, slot, rt)
+	_, err = babeService.buildBlock(parentHeader, slot, rt, 0, &VrfOutputAndProof{})


What's 0 here? Perhaps have it as a named constant above this line? 🤔

qdm12 · 2022-01-25T19:53:25Z

lib/babe/crypto_test.go

@@ -77,7 +77,7 @@ func TestCalculateThreshold(t *testing.T) {
 }

 func Test_checkPrimaryThreshold(t *testing.T) {
-	keyring, _ := keystore.NewSr25519Keyring()
+	keyring, _ := keystore.NewSr25519Keyring() //nolint:govet


Why is govet complaining here 🤔
If it's because of the error, just assert it's nil?

qdm12 · 2022-01-25T20:14:12Z

lib/babe/epoch_handler.go

+			if err := h.handleSlot(h.epochNumber, swt.slotNum, h.epochData.authorityIndex, h.slotToProof[swt.slotNum]); err != nil { //nolint:lll
+				logger.Warnf("failed to handle slot %d: %s", swt.slotNum, err)
+				continue
+			}


nit don't use the short err check form with a big line like this.

Suggested change

if err := h.handleSlot(h.epochNumber, swt.slotNum, h.epochData.authorityIndex, h.slotToProof[swt.slotNum]); err != nil { //nolint:lll

logger.Warnf("failed to handle slot %d: %s", swt.slotNum, err)

continue

}

err := h.handleSlot(h.epochNumber, swt.slotNum,

h.epochData.authorityIndex, h.slotToProof[swt.slotNum])

if err != nil {

logger.Warnf("failed to handle slot %d: %s", swt.slotNum, err)

continue

}

qdm12 · 2022-01-25T20:14:54Z

lib/babe/epoch_handler.go

+	authoringSlots := make([]uint64, len(slotToProof))
+	i := 0
+	for authoringSlot := range slotToProof {
+		authoringSlots[i] = authoringSlot
+		i++


nit simpler

Suggested change

authoringSlots := make([]uint64, len(slotToProof))

i := 0

for authoringSlot := range slotToProof {

authoringSlots[i] = authoringSlot

i++

authoringSlots := make([]uint64, 0, len(slotToProof))

for authoringSlot := range slotToProof {

authoringSlots = append(authoringSlots, authoringSlot)

qdm12 · 2022-01-25T20:16:38Z

lib/babe/epoch_handler_test.go

+	timer := time.After(sd * 100)
+	select {
+	case <-timer:
+		require.Equal(t, 100-(firstExecutedSlot-startSlot), callsToHandleSlot)


nit Have 100 as a shared inline constant please

qdm12 · 2022-01-25T20:17:11Z

lib/babe/epoch_handler_test.go

+
+	errCh := make(chan error)
+	go eh.run(ctx, errCh)
+	timer := time.After(sd * 100)


nit Ideally use time.NewTimer(sd * 100) so you can stop the timer in the error receiving select case.

qdm12 · 2022-01-25T20:17:43Z

lib/babe/epoch_handler_test.go

+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	eh, err := newEpochHandler(1, startSlot, epochData, constants, testHandleSlotFunc, keypair)


nit Please rename eh to epochHandler or handler or any full work 😸

EclesioMeloJunior · 2022-01-25T20:19:31Z

lib/babe/babe.go

+	for {
+		if err = b.handleEpoch(epoch); err != nil {
+			if errors.Is(err, errServicePaused) || errors.Is(err, context.Canceled) {
 				return nil


nit

Suggested change

for {

if err = b.handleEpoch(epoch); err != nil {

if errors.Is(err, errServicePaused) || errors.Is(err, context.Canceled) {

return nil

for {

err = b.handleEpoch(epoch)

if errors.Is(err, errServicePaused) || errors.Is(err, context.Canceled) {

return nil

} else if err != nil {

return err

}

qdm12 · 2022-01-25T20:19:00Z

lib/babe/babe.go

@@ -263,7 +263,9 @@ func (b *Service) Stop() error {
 // Authorities returns the current BABE authorities
 func (b *Service) Authorities() []types.Authority {
 	auths := make([]types.Authority, len(b.epochHandler.epochData.authorities))
-	copy(auths, b.epochHandler.epochData.authorities)
+	for i, auth := range b.epochHandler.epochData.authorities {
+		auths[i] = *(auth.DeepCopy())


nit do you need the parentheses? I think this would work too

Suggested change

auths[i] = *(auth.DeepCopy())

auths[i] = *auth.DeepCopy()

qdm12 · 2022-01-25T20:19:48Z

lib/babe/babe.go

+	defer cancel()
+	b.epochHandler, err = b.initiateAndGetEpochHandler(epoch)
+	if err != nil {
+		return err


Suggested change

return err

return fmt.Errorf("cannot initiate and get epoch handler for epoch %d: %w", epoch, err)

qdm12 · 2022-01-25T20:20:55Z

lib/babe/babe.go

+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()


nit perhaps move this further down before it's needed at go b.epochHandler.run(ctx, errCh)

qdm12 · 2022-01-25T20:22:01Z

lib/babe/babe.go

+		// stop current epoch handler
+		cancel()
+	case err := <-errCh:
+		logger.Errorf("error from epochHandler: %w", err)


Suggested change

logger.Errorf("error from epochHandler: %w", err)

logger.Errorf("error from epochHandler: %s", err)

qdm12 · 2022-01-25T20:23:26Z

lib/babe/babe.go

+	case <-epochTimer.C:
+		// stop current epoch handler
+		cancel()
+	case err := <-errCh:


You should also call cleanup() here

Suggested change

case err := <-errCh:

case err := <-errCh:

cleanup()

noot · 2022-01-27T16:39:40Z

@qdm12 I think I addressed all your comments, feel free to rereview!

noot · 2022-01-27T22:12:21Z

i think this also closes #2217 (don't see the error anymore when running this branch), will try it more to confirm though.

stale

noot added 3 commits January 6, 2022 18:21

refactor handling slots in an epoch into epochHandler

080641c

fix tests, lint

8b331c7

cleanup comments

1d52f2b

noot self-assigned this Jan 7, 2022

noot added 15 commits January 7, 2022 17:46

add license

0beea7f

add some string funcs

525c309

comment out peer scoring

928f76c

lint

5210bf1

Merge branch 'development' of github.com:ChainSafe/gossamer into noot…

ef2676e

…/babe-refactor

merge w development

842f601

fix merge, lint

e13c257

cleanup babe epcoch.go

1d52d1f

fix test setup

a3d3e17

start epoch tests

78cd721

comment out more stuff in peerset tests

5a00c5f

add test for getEpochDataAndStartSlot

928e974

begin epochHandler tests

a3919e7

add epochHandler.run test

f242ea6

lint

48e882e

noot marked this pull request as ready for review January 19, 2022 22:10

noot requested review from EclesioMeloJunior, edwardmack, jimjbrettj, kishansagathiya, qdm12 and timwu20 as code owners January 19, 2022 22:10

cleanup

c00241b

noot commented Jan 19, 2022

View reviewed changes

comment out more stuff

b1bf847

noot requested a review from kishansagathiya January 21, 2022 19:30

fix logic to return if b.ctx is done

a193753

qdm12 reviewed Jan 21, 2022

View reviewed changes

timwu20 reviewed Jan 22, 2022

View reviewed changes

address comments

4eaa134

noot requested review from timwu20 and qdm12 January 24, 2022 18:20

kishansagathiya approved these changes Jan 24, 2022

View reviewed changes

EclesioMeloJunior approved these changes Jan 24, 2022

View reviewed changes

lib/babe/babe.go Outdated Show resolved Hide resolved

lib/babe/babe.go Outdated Show resolved Hide resolved

lib/babe/epoch_handler.go Outdated Show resolved Hide resolved

EclesioMeloJunior requested changes Jan 24, 2022

View reviewed changes

address comments

6bd1a9d

noot requested a review from EclesioMeloJunior January 24, 2022 22:17

qdm12 previously requested changes Jan 24, 2022

View reviewed changes

qdm12 reviewed Jan 24, 2022

View reviewed changes

address comemnts

f350dc9

qdm12 reviewed Jan 25, 2022

View reviewed changes

EclesioMeloJunior approved these changes Jan 25, 2022

View reviewed changes

qdm12 reviewed Jan 25, 2022

View reviewed changes

address comments

d110a8f

noot requested a review from qdm12 January 27, 2022 16:50

fix

a953ff8

noot changed the title ~~maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic~~ maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic~ Jan 28, 2022

noot changed the title ~~maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic~~~ maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic Jan 28, 2022

noot merged commit 656fd8d into development Jan 28, 2022

noot deleted the noot/babe-refactor branch January 28, 2022 19:36

noot mentioned this pull request Jan 31, 2022

cross-client devnet: substrate shouldn't drop us as a peer #2252

Closed

	return nil, fmt.Errorf("failed to initiate epoch %d: %s", epoch, err)
	return nil, fmt.Errorf("failed to initiate epoch %d: %w", epoch, err)

	return nil, fmt.Errorf("failed to get start slot for current epoch %d: %s", epoch, err)
	return nil, fmt.Errorf("failed to get start slot for current epoch %d: %w", epoch, err)

	return fmt.Errorf("failed to get current epoch: %s", err)
	return fmt.Errorf("failed to get current epoch: %w", err)

	return err
	return fmt.Errorf("cannot initiate epoch handler: %w", err)

	return fmt.Errorf("failed to get start slot for next epoch %d: %s", epoch+1, err)
	return fmt.Errorf("failed to get start slot for next epoch %d: %w", epoch+1, err)

		epochData.authorities = make([]types.Authority, 1)
		epochData.authorities[0] = types.Authority{

	epochData.authorities = make([]types.Authority, 1)
	epochData.authorities[0] = types.Authority{
	epochData.authorities = []types.Authority{

	authoringSlots := getAuthoringSlots(babeService.epochHandler.slotToProof)
	authoringSlots := getAuthoringSlots(babeService.epochHandler.slotToProof)
	require.NotEmpty(t, authoringSlots)

	return err
	return fmt.Errorf("cannot initiate and get epoch handler for epoch %d: %w", epoch, err)

		ctx, cancel := context.WithCancel(context.Background())
		defer cancel()

	logger.Errorf("error from epochHandler: %w", err)
	logger.Errorf("error from epochHandler: %s", err)

maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic #2151

maintainence(lib/babe): refactor code to use epochHandler for control of epoch logic #2151

Uh oh!

Conversation

noot commented Jan 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Issues

Primary Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

timwu20 Jan 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noot commented Jan 24, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EclesioMeloJunior left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noot commented Jan 7, 2022 •

edited

Loading

timwu20 Jan 22, 2022 •

edited

Loading

qdm12 Jan 24, 2022 •

edited

Loading