Skip to content

Commit

Permalink
remove block_id from starknet_subscribeTransactionStatus (#2410)
Browse files Browse the repository at this point in the history
* remove block_id from starknet_subscribeTransactionStatus according to RPCv8-rc2
  • Loading branch information
rianhughes authored Jan 31, 2025
1 parent 42077d3 commit 1caf86f
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 57 deletions.
2 changes: 1 addition & 1 deletion rpc/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ func (h *Handler) Methods() ([]jsonrpc.Method, string) { //nolint: funlen
},
{
Name: "starknet_subscribeTransactionStatus",
Params: []jsonrpc.Parameter{{Name: "transaction_hash"}, {Name: "block_id", Optional: true}},
Params: []jsonrpc.Parameter{{Name: "transaction_hash"}},
Handler: h.SubscribeTransactionStatus,
},
{
Expand Down
10 changes: 1 addition & 9 deletions rpc/subscriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,14 @@ func (h *Handler) SubscribeEvents(ctx context.Context, fromAddr *felt.Felt, keys
// The optional block_id parameter is ignored, as status changes are not stored and historical data cannot be sent.
//
//nolint:gocyclo,funlen
func (h *Handler) SubscribeTransactionStatus(ctx context.Context, txHash felt.Felt, blockID *BlockID) (*SubscriptionID,
func (h *Handler) SubscribeTransactionStatus(ctx context.Context, txHash felt.Felt) (*SubscriptionID,
*jsonrpc.Error,
) {
w, ok := jsonrpc.ConnFromContext(ctx)
if !ok {
return nil, jsonrpc.Err(jsonrpc.MethodNotFound, nil)
}

// resolveBlockRange is only used to make sure that the requested block id is not older than 1024 block and check
// if the requested block is found. The range is inconsequential since we assume the provided transaction hash
// of a transaction is included in the block range: latest/pending - 1024.
_, _, rpcErr := h.resolveBlockRange(blockID)
if rpcErr != nil {
return nil, rpcErr
}

// If the error is transaction not found that means the transaction has not been submitted to the feeder gateway,
// therefore, we need to wait for a specified time and at regular interval check if the transaction has been found.
// If the transaction is found during the timout expiry, then we continue to keep track of its status otherwise the
Expand Down
51 changes: 4 additions & 47 deletions rpc/subscriptions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ func TestSubscribeTxnStatus(t *testing.T) {
mockSyncer := mocks.NewMockSyncReader(mockCtrl)
handler := New(mockChain, mockSyncer, nil, "", log)

mockChain.EXPECT().HeadsHeader().Return(&core.Header{Number: 1}, nil)
mockChain.EXPECT().TransactionByHash(txHash).Return(nil, db.ErrKeyNotFound).AnyTimes()
mockSyncer.EXPECT().PendingBlock().Return(nil).AnyTimes()

Expand All @@ -352,50 +351,11 @@ func TestSubscribeTxnStatus(t *testing.T) {

subCtx := context.WithValue(context.Background(), jsonrpc.ConnKey{}, &fakeConn{w: serverConn})

id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash, nil)
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash)
assert.Nil(t, id)
assert.Equal(t, ErrTxnHashNotFound, rpcErr)
})

t.Run("Return error if block is too far back", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
t.Cleanup(mockCtrl.Finish)

mockChain := mocks.NewMockReader(mockCtrl)
mockSyncer := mocks.NewMockSyncReader(mockCtrl)
handler := New(mockChain, mockSyncer, nil, "", log)

blockID := &BlockID{Number: 0}

serverConn, _ := net.Pipe()
t.Cleanup(func() {
require.NoError(t, serverConn.Close())
})

subCtx := context.WithValue(context.Background(), jsonrpc.ConnKey{}, &fakeConn{w: serverConn})

// Note the end of the window doesn't need to be tested because if requested block number is more than the
// head, a block not found error will be returned. This behaviour has been tested in various other tests, and we
// don't need to test it here again.
t.Run("head is 1024", func(t *testing.T) {
mockChain.EXPECT().HeadsHeader().Return(&core.Header{Number: 1024}, nil)
mockChain.EXPECT().BlockHeaderByNumber(blockID.Number).Return(&core.Header{Number: 0}, nil)

id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash, blockID)
assert.Zero(t, id)
assert.Equal(t, ErrTooManyBlocksBack, rpcErr)
})

t.Run("head is more than 1024", func(t *testing.T) {
mockChain.EXPECT().HeadsHeader().Return(&core.Header{Number: 2024}, nil)
mockChain.EXPECT().BlockHeaderByNumber(blockID.Number).Return(&core.Header{Number: 0}, nil)

id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash, blockID)
assert.Zero(t, id)
assert.Equal(t, ErrTooManyBlocksBack, rpcErr)
})
})

t.Run("Transaction status is final", func(t *testing.T) {
mockCtrl := gomock.NewController(t)
t.Cleanup(mockCtrl.Finish)
Expand All @@ -416,13 +376,12 @@ func TestSubscribeTxnStatus(t *testing.T) {
txHash, err := new(felt.Felt).SetString("0x1111")
require.NoError(t, err)

mockChain.EXPECT().HeadsHeader().Return(&core.Header{Number: 1}, nil)
mockChain.EXPECT().TransactionByHash(txHash).Return(nil, db.ErrKeyNotFound)
mockSyncer.EXPECT().PendingBlock().Return(nil)

ctx, cancel := context.WithCancel(context.Background())
subCtx := context.WithValue(ctx, jsonrpc.ConnKey{}, &fakeConn{w: serverConn})
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash, nil)
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash)
require.Nil(t, rpcErr)

b, err := TxnStatusRejected.MarshalText()
Expand All @@ -447,13 +406,12 @@ func TestSubscribeTxnStatus(t *testing.T) {
txHash, err := new(felt.Felt).SetString("0x1010")
require.NoError(t, err)

mockChain.EXPECT().HeadsHeader().Return(&core.Header{Number: 1}, nil)
mockChain.EXPECT().TransactionByHash(txHash).Return(nil, db.ErrKeyNotFound)
mockSyncer.EXPECT().PendingBlock().Return(nil)

ctx, cancel := context.WithCancel(context.Background())
subCtx := context.WithValue(ctx, jsonrpc.ConnKey{}, &fakeConn{w: serverConn})
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash, nil)
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash)
require.Nil(t, rpcErr)

b, err := TxnStatusAcceptedOnL1.MarshalText()
Expand Down Expand Up @@ -496,13 +454,12 @@ func TestSubscribeTxnStatus(t *testing.T) {
txHash, err := new(felt.Felt).SetString("0x1001")
require.NoError(t, err)

mockChain.EXPECT().HeadsHeader().Return(&core.Header{Number: block.Number}, nil)
mockChain.EXPECT().TransactionByHash(txHash).Return(nil, db.ErrKeyNotFound)
mockSyncer.EXPECT().PendingBlock().Return(nil)

ctx, cancel := context.WithCancel(context.Background())
subCtx := context.WithValue(ctx, jsonrpc.ConnKey{}, &fakeConn{w: serverConn})
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash, nil)
id, rpcErr := handler.SubscribeTransactionStatus(subCtx, *txHash)
require.Nil(t, rpcErr)

b, err := TxnStatusReceived.MarshalText()
Expand Down

0 comments on commit 1caf86f

Please sign in to comment.