Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Reuse v6 starknet_getTransactionByHash handler for v7 #2486

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rpc/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func (h *Handler) MethodsV0_7() ([]jsonrpc.Method, string) { //nolint: funlen
{
Name: "starknet_getTransactionByHash",
Params: []jsonrpc.Parameter{{Name: "transaction_hash"}},
Handler: h.rpcv7Handler.TransactionByHash,
Handler: h.rpcv6Handler.TransactionByHash,
},
{
Name: "starknet_getTransactionReceipt",
Expand Down
27 changes: 24 additions & 3 deletions rpc/v6/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"github.com/NethermindEth/juno/clients/gateway"
"github.com/NethermindEth/juno/core"
"github.com/NethermindEth/juno/core/felt"
"github.com/NethermindEth/juno/db"
"github.com/NethermindEth/juno/jsonrpc"
rpccore "github.com/NethermindEth/juno/rpc/rpccore"
"github.com/NethermindEth/juno/starknet"
Expand Down Expand Up @@ -427,8 +428,28 @@
func (h *Handler) TransactionByHash(hash felt.Felt) (*Transaction, *jsonrpc.Error) {
txn, err := h.bcReader.TransactionByHash(&hash)
if err != nil {
return nil, rpccore.ErrTxnHashNotFound
if !errors.Is(err, db.ErrKeyNotFound) {
return nil, rpccore.ErrInternal.CloneWithData(err)
}

Check warning on line 433 in rpc/v6/transaction.go

View check run for this annotation

Codecov / codecov/patch

rpc/v6/transaction.go#L432-L433

Added lines #L432 - L433 were not covered by tests

// check now if tx is in pending block
pendingB := h.syncReader.PendingBlock()
if pendingB == nil {
return nil, rpccore.ErrTxnHashNotFound
}

Check warning on line 439 in rpc/v6/transaction.go

View check run for this annotation

Codecov / codecov/patch

rpc/v6/transaction.go#L438-L439

Added lines #L438 - L439 were not covered by tests

for _, t := range pendingB.Transactions {
if hash.Equal(t.Hash()) {
txn = t
break

Check warning on line 444 in rpc/v6/transaction.go

View check run for this annotation

Codecov / codecov/patch

rpc/v6/transaction.go#L443-L444

Added lines #L443 - L444 were not covered by tests
}
}

if txn == nil {
return nil, rpccore.ErrTxnHashNotFound
}
}

return AdaptTransaction(txn), nil
}

Expand Down Expand Up @@ -665,7 +686,7 @@
case *core.DeclareTransaction:
txn = adaptDeclareTransaction(v)
case *core.DeployAccountTransaction:
txn = adaptDeployAccountTrandaction(v)
txn = adaptDeployAccountTransaction(v)
case *core.L1HandlerTransaction:
nonce := v.Nonce
if nonce == nil {
Expand Down Expand Up @@ -809,7 +830,7 @@
return tx
}

func adaptDeployAccountTrandaction(t *core.DeployAccountTransaction) *Transaction {
func adaptDeployAccountTransaction(t *core.DeployAccountTransaction) *Transaction {
tx := &Transaction{
Hash: t.Hash(),
MaxFee: t.MaxFee,
Expand Down
16 changes: 12 additions & 4 deletions rpc/v6/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,22 @@ func TestTransactionByHashNotFound(t *testing.T) {
mockCtrl := gomock.NewController(t)
t.Cleanup(mockCtrl.Finish)
mockReader := mocks.NewMockReader(mockCtrl)
mockSyncReader := mocks.NewMockSyncReader(mockCtrl)

n := utils.Ptr(utils.Mainnet)
txHash := new(felt.Felt).SetBytes([]byte("random hash"))
mockReader.EXPECT().TransactionByHash(txHash).Return(nil, errors.New("tx not found"))
client := feeder.NewTestClient(t, n)
mainnetGw := adaptfeeder.New(client)

handler := rpc.New(mockReader, nil, nil, "", n, nil)
block, err := mainnetGw.BlockByNumber(context.Background(), 19199)
require.NoError(t, err)

randomTxHash := new(felt.Felt).SetBytes([]byte("random hash"))
mockReader.EXPECT().TransactionByHash(randomTxHash).Return(nil, db.ErrKeyNotFound)
mockSyncReader.EXPECT().PendingBlock().Return(block)

handler := rpc.New(mockReader, mockSyncReader, nil, "", n, nil)
tx, rpcErr := handler.TransactionByHash(*randomTxHash)

tx, rpcErr := handler.TransactionByHash(*txHash)
assert.Nil(t, tx)
assert.Equal(t, rpccore.ErrTxnHashNotFound, rpcErr)
}
Expand Down
11 changes: 8 additions & 3 deletions rpc/v7/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,11 @@ func TestBlockWithTxs(t *testing.T) {
n := utils.Ptr(utils.Mainnet)
handler := rpcv7.New(mockReader, mockSyncReader, nil, "", n, nil)

// Use v6 handler as v7 `starknet_getTransactionByHash` uses v6 handler
mockSyncReaderV6 := mocks.NewMockSyncReader(mockCtrl)
mockReaderV6 := mocks.NewMockReader(mockCtrl)
rpcv6Handler := rpcv6.New(mockReaderV6, mockSyncReaderV6, nil, "", n, nil)

client := feeder.NewTestClient(t, n)
gw := adaptfeeder.New(client)

Expand All @@ -267,10 +272,10 @@ func TestBlockWithTxs(t *testing.T) {
assert.Equal(t, len(blockWithTxHashes.TxnHashes), len(blockWithTxs.Transactions))

for i, txnHash := range blockWithTxHashes.TxnHashes {
txn, err := handler.TransactionByHash(*txnHash)
txn, err := rpcv6Handler.TransactionByHash(*txnHash)
require.Nil(t, err)

assert.Equal(t, txn, blockWithTxs.Transactions[i])
assert.Equal(t, adaptV6TxToV7(t, txn), blockWithTxs.Transactions[i])
}
}

Expand All @@ -279,7 +284,7 @@ func TestBlockWithTxs(t *testing.T) {
latestBlockTxMap[*tx.Hash()] = tx
}

mockReader.EXPECT().TransactionByHash(gomock.Any()).DoAndReturn(func(hash *felt.Felt) (core.Transaction, error) {
mockReaderV6.EXPECT().TransactionByHash(gomock.Any()).DoAndReturn(func(hash *felt.Felt) (core.Transaction, error) {
if tx, found := latestBlockTxMap[*hash]; found {
return tx, nil
}
Expand Down
30 changes: 0 additions & 30 deletions rpc/v7/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,36 +422,6 @@ func adaptRPCTxToFeederTx(rpcTx *Transaction) *starknet.Transaction {
Transaction Handlers
*****************************************************/

// TransactionByHash returns the details of a transaction identified by the given hash.
//
// It follows the specification defined here:
// https://github.com/starkware-libs/starknet-specs/blob/master/api/starknet_api_openrpc.json#L158
func (h *Handler) TransactionByHash(hash felt.Felt) (*Transaction, *jsonrpc.Error) {
txn, err := h.bcReader.TransactionByHash(&hash)
if err != nil {
if !errors.Is(err, db.ErrKeyNotFound) {
return nil, rpccore.ErrInternal.CloneWithData(err)
}

pendingB := h.syncReader.PendingBlock()
if pendingB == nil {
return nil, rpccore.ErrTxnHashNotFound
}

for _, t := range pendingB.Transactions {
if hash.Equal(t.Hash()) {
txn = t
break
}
}

if txn == nil {
return nil, rpccore.ErrTxnHashNotFound
}
}
return AdaptTransaction(txn), nil
}

// TransactionByBlockIDAndIndex returns the details of a transaction identified by the given
// BlockID and index.
//
Expand Down
Loading
Loading