From dea11339aa9962b352196e86e9ce621c516af594 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 26 Jun 2024 08:47:07 +0200 Subject: [PATCH] chore: review improvements --- pkg/client/interface.go | 8 ++++---- pkg/client/query/sharedquerier.go | 8 ++++---- pkg/relayer/session/claim.go | 2 +- pkg/relayer/session/proof.go | 7 +++---- x/proof/keeper/msg_server_submit_proof.go | 12 ++++++++---- x/proof/types/shared_query_client.go | 12 ++++++------ x/shared/session.go | 8 ++++---- x/shared/session_test.go | 8 ++++---- 8 files changed, 34 insertions(+), 31 deletions(-) diff --git a/pkg/client/interface.go b/pkg/client/interface.go index 93dcfafc4..d1b712696 100644 --- a/pkg/client/interface.go +++ b/pkg/client/interface.go @@ -294,15 +294,15 @@ type SharedQueryClient interface { // GetClaimWindowOpenHeight returns the block height at which the claim window of // the session that includes queryHeight opens. GetClaimWindowOpenHeight(ctx context.Context, queryHeight int64) (int64, error) - // GetEarliestClaimCommitHeight returns the earliest block height at which a claim + // GetEarliestSupplierClaimCommitHeight returns the earliest block height at which a claim // for the session that includes queryHeight can be committed for a given supplier. - GetEarliestClaimCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) + GetEarliestSupplierClaimCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) // GetProofWindowOpenHeight returns the block height at which the proof window of // the session that includes queryHeight opens. GetProofWindowOpenHeight(ctx context.Context, queryHeight int64) (int64, error) - // GetEarliestProofCommitHeight returns the earliest block height at which a proof + // GetEarliestSupplierProofCommitHeight returns the earliest block height at which a proof // for the session that includes queryHeight can be committed for a given supplier. - GetEarliestProofCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) + GetEarliestSupplierProofCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) } // BlockQueryClient defines an interface that enables the querying of diff --git a/pkg/client/query/sharedquerier.go b/pkg/client/query/sharedquerier.go index 1d45045b1..3a31404af 100644 --- a/pkg/client/query/sharedquerier.go +++ b/pkg/client/query/sharedquerier.go @@ -117,7 +117,7 @@ func (sq *sharedQuerier) GetSessionGracePeriodEndHeight( // to get the most recently (asynchronously) observed (and cached) value. // TODO_BLOCKER(@bryanchriswhite, #543): We also don't really want to use the current value of the params. // Instead, we should be using the value that the params had for the session which includes queryHeight. -func (sq *sharedQuerier) GetEarliestClaimCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) { +func (sq *sharedQuerier) GetEarliestSupplierClaimCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) { sharedParams, err := sq.GetParams(ctx) if err != nil { return 0, err @@ -134,7 +134,7 @@ func (sq *sharedQuerier) GetEarliestClaimCommitHeight(ctx context.Context, query // NB: Byte slice representation of block hashes don't need to be normalized. claimWindowOpenBlockHash := claimWindowOpenBlock.BlockID.Hash.Bytes() - return shared.GetEarliestClaimCommitHeight( + return shared.GetEarliestSupplierClaimCommitHeight( sharedParams, queryHeight, claimWindowOpenBlockHash, @@ -150,7 +150,7 @@ func (sq *sharedQuerier) GetEarliestClaimCommitHeight(ctx context.Context, query // to get the most recently (asynchronously) observed (and cached) value. // TODO_BLOCKER(@bryanchriswhite, #543): We also don't really want to use the current value of the params. // Instead, we should be using the value that the params had for the session which includes queryHeight. -func (sq *sharedQuerier) GetEarliestProofCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) { +func (sq *sharedQuerier) GetEarliestSupplierProofCommitHeight(ctx context.Context, queryHeight int64, supplierAddr string) (int64, error) { sharedParams, err := sq.GetParams(ctx) if err != nil { return 0, err @@ -164,7 +164,7 @@ func (sq *sharedQuerier) GetEarliestProofCommitHeight(ctx context.Context, query return 0, err } - return shared.GetEarliestProofCommitHeight( + return shared.GetEarliestSupplierProofCommitHeight( sharedParams, queryHeight, proofWindowOpenBlock.BlockID.Hash, diff --git a/pkg/relayer/session/claim.go b/pkg/relayer/session/claim.go index 88ef047da..774f0a577 100644 --- a/pkg/relayer/session/claim.go +++ b/pkg/relayer/session/claim.go @@ -120,7 +120,7 @@ func (rs *relayerSessionsManager) waitForEarliestCreateClaimsHeight( // Get the earliestClaimsCommitHeight. supplierAddr := sessionTrees[0].GetSupplierAddress().String() - earliestClaimsCommitHeight, err := rs.sharedQueryClient.GetEarliestClaimCommitHeight(ctx, sessionEndHeight, supplierAddr) + earliestClaimsCommitHeight, err := rs.sharedQueryClient.GetEarliestSupplierClaimCommitHeight(ctx, sessionEndHeight, supplierAddr) if err != nil { failedCreateClaimsSessionsCh <- sessionTrees return nil diff --git a/pkg/relayer/session/proof.go b/pkg/relayer/session/proof.go index bc8ee495b..02fab6130 100644 --- a/pkg/relayer/session/proof.go +++ b/pkg/relayer/session/proof.go @@ -98,11 +98,10 @@ func (rs *relayerSessionsManager) waitForEarliestSubmitProofsHeightAndGeneratePr logger = logger.With("proof_window_open_height", proofWindowOpenHeight) logger.Info().Msg("waiting & blocking for proof path seed block height") - // proofPathSeedBlock is the block that will have its hash used as the + // proofWindowOpenHeight - 1 is the block that will have its hash used as the // source of entropy for all the session trees in that batch, waiting for it to // be received before proceeding. - proofPathSeedBlockHeight := shared.GetSessionGracePeriodEndHeight(sharedParams, sessionEndHeight) - proofPathSeedBlock := rs.waitForBlock(ctx, proofPathSeedBlockHeight) + proofPathSeedBlock := rs.waitForBlock(ctx, proofWindowOpenHeight-1) _ = rs.waitForBlock(ctx, proofWindowOpenHeight) logger = logger.With("proof_path_bock_hash", fmt.Sprintf("%x", proofPathSeedBlock.Hash())) @@ -123,7 +122,7 @@ func (rs *relayerSessionsManager) waitForEarliestSubmitProofsHeightAndGeneratePr // Get the earliestProofsCommitHeight. supplierAddr := sessionTrees[0].GetSupplierAddress() - earliestProofsCommitHeight, err := rs.sharedQueryClient.GetEarliestProofCommitHeight(ctx, sessionEndHeight, supplierAddr.String()) + earliestProofsCommitHeight, err := rs.sharedQueryClient.GetEarliestSupplierProofCommitHeight(ctx, sessionEndHeight, supplierAddr.String()) if err != nil { failedSubmitProofsSessionsCh <- sessionTrees return nil diff --git a/x/proof/keeper/msg_server_submit_proof.go b/x/proof/keeper/msg_server_submit_proof.go index 8cccc80dd..7b461d869 100644 --- a/x/proof/keeper/msg_server_submit_proof.go +++ b/x/proof/keeper/msg_server_submit_proof.go @@ -463,17 +463,21 @@ func (k msgServer) validateClosestPath( // // Since smt.ProveClosest is defined in terms of submitProofWindowStartHeight, // this block's hash needs to be used for validation too. - sessionGracePeriodEndHeight, err := k.sharedQuerier.GetSessionGracePeriodEndHeight(ctx, sessionHeader.GetSessionEndBlockHeight()) + proofWindowOpenHeight, err := k.sharedQuerier.GetProofWindowOpenHeight(ctx, sessionHeader.GetSessionEndBlockHeight()) if err != nil { return err } - blockHash := k.sessionKeeper.GetBlockHash(ctx, sessionGracePeriodEndHeight) + + // proofWindowOpenHeight - 1 is the block that will have its hash used as the + // source of entropy for all the session trees in that batch, waiting for it to + // be received before proceeding. + proofPathSeedBlockHash := k.sessionKeeper.GetBlockHash(ctx, proofWindowOpenHeight-1) // TODO_BETA: Investigate "proof for the path provided does not match one expected by the on-chain protocol" // error that may occur due to block height differing from the off-chain part. - fmt.Println("E2E_DEBUG: height for block hash when verifying the proof", sessionGracePeriodEndHeight, sessionHeader.GetSessionId()) + k.logger.Info("E2E_DEBUG: height for block hash when verifying the proof", proofWindowOpenHeight, sessionHeader.GetSessionId()) - expectedProofPath := GetPathForProof(blockHash, sessionHeader.GetSessionId()) + expectedProofPath := GetPathForProof(proofPathSeedBlockHash, sessionHeader.GetSessionId()) if !bytes.Equal(proof.Path, expectedProofPath) { return types.ErrProofInvalidProof.Wrapf( "the proof for the path provided (%x) does not match one expected by the on-chain protocol (%x)", diff --git a/x/proof/types/shared_query_client.go b/x/proof/types/shared_query_client.go index 1ba69c5b2..38cc635e5 100644 --- a/x/proof/types/shared_query_client.go +++ b/x/proof/types/shared_query_client.go @@ -77,9 +77,9 @@ func (sqc *SharedKeeperQueryClient) GetProofWindowOpenHeight( return shared.GetProofWindowOpenHeight(&sharedParams, queryHeight), nil } -// GetEarliestClaimCommitHeight returns the earliest block height at which a claim +// GetEarliestSupplierClaimCommitHeight returns the earliest block height at which a claim // for the session that includes queryHeight can be committed for a given supplier. -func (sqc *SharedKeeperQueryClient) GetEarliestClaimCommitHeight( +func (sqc *SharedKeeperQueryClient) GetEarliestSupplierClaimCommitHeight( ctx context.Context, queryHeight int64, supplierAddr string, @@ -93,7 +93,7 @@ func (sqc *SharedKeeperQueryClient) GetEarliestClaimCommitHeight( claimWindowOpenBlockHashBz := sqc.sessionKeeper.GetBlockHash(ctx, claimWindowOpenHeight) // Get the earliest claim commit height for the given supplier. - return shared.GetEarliestClaimCommitHeight( + return shared.GetEarliestSupplierClaimCommitHeight( &sharedParams, queryHeight, claimWindowOpenBlockHashBz, @@ -101,9 +101,9 @@ func (sqc *SharedKeeperQueryClient) GetEarliestClaimCommitHeight( ), nil } -// GetEarliestProofCommitHeight returns the earliest block height at which a proof +// GetEarliestSupplierProofCommitHeight returns the earliest block height at which a proof // for the session that includes queryHeight can be committed for a given supplier. -func (sqc *SharedKeeperQueryClient) GetEarliestProofCommitHeight( +func (sqc *SharedKeeperQueryClient) GetEarliestSupplierProofCommitHeight( ctx context.Context, queryHeight int64, supplierAddr string, @@ -117,7 +117,7 @@ func (sqc *SharedKeeperQueryClient) GetEarliestProofCommitHeight( proofWindowOpenBlockHash := sqc.sessionKeeper.GetBlockHash(ctx, proofWindowOpenHeight) // Get the earliest proof commit height for the given supplier. - return shared.GetEarliestProofCommitHeight( + return shared.GetEarliestSupplierProofCommitHeight( &sharedParams, queryHeight, proofWindowOpenBlockHash, diff --git a/x/shared/session.go b/x/shared/session.go index deba0806b..4a36992de 100644 --- a/x/shared/session.go +++ b/x/shared/session.go @@ -113,10 +113,10 @@ func GetProofWindowCloseHeight(sharedParams *sharedtypes.Params, queryHeight int int64(sharedParams.GetProofWindowCloseOffsetBlocks()) } -// GetEarliestClaimCommitHeight returns the earliest block height at which a claim +// GetEarliestSupplierClaimCommitHeight returns the earliest block height at which a claim // for the session that includes queryHeight can be committed for a given supplier // and the passed sharedParams. -func GetEarliestClaimCommitHeight( +func GetEarliestSupplierClaimCommitHeight( sharedParams *sharedtypes.Params, queryHeight int64, claimWindowOpenBlockHash []byte, @@ -148,10 +148,10 @@ func GetClaimWindowSizeBlocks(sharedParams *sharedtypes.Params) uint64 { return windowSizeBlocks } -// GetEarliestProofCommitHeight returns the earliest block height at which a proof +// GetEarliestSupplierProofCommitHeight returns the earliest block height at which a proof // for the session that includes queryHeight can be committed for a given supplier // and the passed sharedParams. -func GetEarliestProofCommitHeight( +func GetEarliestSupplierProofCommitHeight( sharedParams *sharedtypes.Params, queryHeight int64, proofWindowOpenBlockHash []byte, diff --git a/x/shared/session_test.go b/x/shared/session_test.go index 0ff52ffa1..0e88fe3bb 100644 --- a/x/shared/session_test.go +++ b/x/shared/session_test.go @@ -19,7 +19,7 @@ func TestGetEarliestClaimCommitHeight_IsDeterministic(t *testing.T) { ) test := func() int64 { - return GetEarliestClaimCommitHeight( + return GetEarliestSupplierClaimCommitHeight( &sharedParams, queryHeight, claimWindowOpenBlockHash[:], @@ -54,7 +54,7 @@ func TestGetEarliestProofCommitHeight_IsDeterministic(t *testing.T) { ) test := func() int64 { - return GetEarliestProofCommitHeight( + return GetEarliestSupplierProofCommitHeight( &sharedParams, queryHeight, proofWindowOpenBlockHash[:], @@ -127,7 +127,7 @@ func TestClaimProofWindows(t *testing.T) { require.GreaterOrEqual(t, proofWindowOpenHeight, claimWindowCloseHeight) require.Greater(t, proofWindowCloseHeight, proofWindowOpenHeight) - earliestClaimCommitHeight := GetEarliestClaimCommitHeight( + earliestClaimCommitHeight := GetEarliestSupplierClaimCommitHeight( &test.sharedParams, test.queryHeight, blockHash, @@ -136,7 +136,7 @@ func TestClaimProofWindows(t *testing.T) { require.Greater(t, claimWindowCloseHeight, earliestClaimCommitHeight) - earliestProofCommitHeight := GetEarliestProofCommitHeight( + earliestProofCommitHeight := GetEarliestSupplierProofCommitHeight( &test.sharedParams, test.queryHeight, blockHash,