diff --git a/e2e/tests/session.feature b/e2e/tests/session.feature index 0a1f9381c..f0626011d 100644 --- a/e2e/tests/session.feature +++ b/e2e/tests/session.feature @@ -5,14 +5,16 @@ Feature: Session Namespace Scenario: Supplier completes claim/proof lifecycle for a valid session Given the user has the pocketd binary installed When the supplier "supplier1" has serviced a session with "5" relays for service "svc1" for application "app1" - And the user should wait for "5" seconds + # TODO_TECHDEBT: Once the session grace period is configurable, set it to 0 at the beginning of this test. + # The timeout for when a claim can be submitted on-chain depends on `createClaimWindowStartHeight`, which + # is a function of `SessionGracePeriod`. The higher this value, the higher this timeout needs to be. Since + # this test is not dependant on the grace period, setting it to 0 and having a lower grace period will simplify it. + And the user should wait for "7" seconds Then the claim created by supplier "supplier1" for service "svc1" for application "app1" should be persisted on-chain -# TODO_IMPROVE: ... -# And an event should be emitted... + # TODO_IMPROVE: And an event should be emitted... And after the supplier submits a proof for the session for service "svc1" for application "app1" Then the proof submitted by supplier "supplier1" for service "svc1" for application "app1" should be persisted on-chain -# TODO_IMPROVE: ... -# And an event should be emitted... + # TODO_IMPROVE: And an event should be emitted... # TODO_BLOCKER(@red-0ne): Make sure to implement and validate this test # One way to exercise this behavior is to close the `RelayMiner` port to prevent diff --git a/go.mod b/go.mod index 3bba35590..d8360ef37 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,6 @@ require ( github.com/athanorlabs/go-dleq v0.1.0 github.com/cometbft/cometbft v0.37.2 github.com/cometbft/cometbft-db v0.8.0 - github.com/cosmos/cosmos-proto v1.0.0-beta.2 github.com/cosmos/cosmos-sdk v0.47.5 github.com/cosmos/gogoproto v1.4.11 github.com/cosmos/ibc-go/v7 v7.3.1 @@ -51,7 +50,6 @@ require ( golang.org/x/crypto v0.15.0 golang.org/x/exp v0.0.0-20230905200255-921286631fa9 golang.org/x/sync v0.5.0 - google.golang.org/genproto/googleapis/api v0.0.0-20230913181813-007df8e322eb google.golang.org/grpc v1.59.0 gopkg.in/yaml.v2 v2.4.0 ) @@ -98,6 +96,7 @@ require ( github.com/containerd/cgroups v1.1.0 // indirect github.com/coreos/go-systemd/v22 v22.5.0 // indirect github.com/cosmos/btcutil v1.0.5 // indirect + github.com/cosmos/cosmos-proto v1.0.0-beta.2 // indirect github.com/cosmos/go-bip39 v1.0.0 // indirect github.com/cosmos/gogogateway v1.2.0 // indirect github.com/cosmos/iavl v0.20.0 // indirect @@ -294,6 +293,7 @@ require ( google.golang.org/api v0.143.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/genproto v0.0.0-20230913181813-007df8e322eb // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20230913181813-007df8e322eb // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20230920204549-e6e6cdab5c13 // indirect google.golang.org/protobuf v1.31.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect diff --git a/x/session/keeper/keeper.go b/x/session/keeper/keeper.go index 3bae5c617..52c5e42d3 100644 --- a/x/session/keeper/keeper.go +++ b/x/session/keeper/keeper.go @@ -60,12 +60,12 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { func (k Keeper) BeginBlocker(ctx sdk.Context) { // ctx.BlockHeader().LastBlockId.Hash is the hash of the last block committed hash := ctx.BlockHeader().LastBlockId.Hash - // ctx.BlockHeader().Height is the height of the current block being processed, - // decrementing it by 1 gives us the height of the last block committed. - height := ctx.BlockHeader().Height - 1 - // If height is 0, ctx.BlockHeader().LastBlockId.Hash will be nil, - // fall back to an empty byte slice. - if height == 0 { + // ctx.BlockHeader().Height is the height of the last committed block. + height := ctx.BlockHeader().Height + // Block height 1 is the first committed block which uses `genesis.json` as its parent. + // See the explanation here for more details: https://github.com/pokt-network/poktroll/issues/377#issuecomment-1936607294 + // Fallback to an empty byte slice during the genesis block. + if height == 1 { hash = []byte{} } diff --git a/x/session/keeper/query_get_session.go b/x/session/keeper/query_get_session.go index cce44fc2c..eac1345a1 100644 --- a/x/session/keeper/query_get_session.go +++ b/x/session/keeper/query_get_session.go @@ -10,6 +10,8 @@ import ( "github.com/pokt-network/poktroll/x/session/types" ) +// GetSession should be deterministic and always return the same session for +// the same block height. func (k Keeper) GetSession(goCtx context.Context, req *types.QueryGetSessionRequest) (*types.QueryGetSessionResponse, error) { if req == nil { return nil, status.Error(codes.InvalidArgument, "invalid request") @@ -21,16 +23,14 @@ func (k Keeper) GetSession(goCtx context.Context, req *types.QueryGetSessionRequ ctx := sdk.UnwrapSDKContext(goCtx) - // If block height is not specified, use the current (context's latest) block height // Note that `GetSession` is called via the `Query` service rather than the `Msg` server. // The former is stateful but does not lead to state transitions, while the latter one // does. The request height depends on how much the node has synched and only acts as a read, // while the `Msg` server handles the code flow of the validator/sequencer when a new block // is being proposed. blockHeight := req.BlockHeight - if blockHeight == 0 { - blockHeight = ctx.BlockHeight() - } + + k.Logger(ctx).Info("Getting session for height: %d", blockHeight) sessionHydrator := NewSessionHydrator(req.ApplicationAddress, req.Service.Id, blockHeight) session, err := k.HydrateSession(ctx, sessionHydrator) diff --git a/x/session/keeper/query_get_session_test.go b/x/session/keeper/query_get_session_test.go index 8ae251e45..9cd574c51 100644 --- a/x/session/keeper/query_get_session_test.go +++ b/x/session/keeper/query_get_session_test.go @@ -26,6 +26,8 @@ func TestSession_GetSession_Success(t *testing.T) { ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors wctx := sdk.WrapSDKContext(ctx) + // TODO_TECHDEBT(#377): These test assume that the genesis block has a height of 0, + // rewrite them in terms of height = 1 genesis. type test struct { name string diff --git a/x/session/keeper/session_hydrator.go b/x/session/keeper/session_hydrator.go index 5d876a0e2..6f081fb3c 100644 --- a/x/session/keeper/session_hydrator.go +++ b/x/session/keeper/session_hydrator.go @@ -16,6 +16,10 @@ import ( sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) +// TODO_TECHDEBT(#377): The business logic in this file assume that genesis has +// a block height of 0. Revisit it and adjust, where/if necessary, accounting for the +// fact that it's 1. + var SHA3HashLen = crypto.SHA3_256.Size() // TODO_BLOCKER(#21): Make these configurable governance param @@ -96,7 +100,11 @@ func (k Keeper) hydrateSessionMetadata(ctx sdk.Context, sh *sessionHydrator) err // TODO_TECHDEBT: Add a test if `blockHeight` is ahead of the current chain or what this node is aware of if sh.blockHeight > ctx.BlockHeight() { - return sdkerrors.Wrapf(types.ErrSessionHydration, "block height %d is ahead of the current block height %d", sh.blockHeight, ctx.BlockHeight()) + return sdkerrors.Wrapf( + types.ErrSessionHydration, + "block height %d is ahead of the last committed block height %d", + sh.blockHeight, ctx.BlockHeight(), + ) } sh.session.NumBlocksPerSession = NumBlocksPerSession diff --git a/x/session/keeper/session_hydrator_test.go b/x/session/keeper/session_hydrator_test.go index 9a6e07781..e914ca0ea 100644 --- a/x/session/keeper/session_hydrator_test.go +++ b/x/session/keeper/session_hydrator_test.go @@ -11,6 +11,9 @@ import ( "github.com/pokt-network/poktroll/x/session/types" ) +// TODO_TECHDEBT(#377): All the tests in this file assume genesis has a block +// height of 0. Rewrite them in terms of height = 1 genesis. + func TestSession_HydrateSession_Success_BaseCase(t *testing.T) { sessionKeeper, ctx := keepertest.SessionKeeper(t) ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors