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

Fix bugs discoverd by e2e tests #2407

Closed
wants to merge 8 commits into from
Closed
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
1 change: 0 additions & 1 deletion adapters/core2p2p/receipt.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ func AdaptExecutionResources(er *core.ExecutionResources) *gen.Receipt_Execution
var l1Gas, l1DataGas, l2Gas, totalL1Gas, totalL1DataGas *felt.Felt
if da := er.DataAvailability; da != nil { // todo(kirill) check that it might be null
l1Gas = new(felt.Felt).SetUint64(da.L1Gas)
l2Gas = new(felt.Felt).SetUint64(da.L2Gas)
l1DataGas = new(felt.Felt).SetUint64(da.L1DataGas)
}
if tgs := er.TotalGasConsumed; tgs != nil {
Expand Down
1 change: 0 additions & 1 deletion adapters/p2p2core/receipt.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func adaptExecutionResources(er *gen.Receipt_ExecutionResources) *core.Execution
},
DataAvailability: &core.DataAvailability{
L1Gas: feltToUint64(er.L1Gas),
L2Gas: feltToUint64(er.L2Gas),
L1DataGas: feltToUint64(er.L1DataGas),
},
MemoryHoles: uint64(er.MemoryHoles),
Expand Down
1 change: 0 additions & 1 deletion core/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ type ExecutionResources struct {
type DataAvailability struct {
L1Gas uint64
L1DataGas uint64
L2Gas uint64
}

type BuiltinInstanceCounter struct {
Expand Down
35 changes: 33 additions & 2 deletions rpc/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
Timestamp uint64 `json:"timestamp"`
SequencerAddress *felt.Felt `json:"sequencer_address,omitempty"`
L1GasPrice *ResourcePrice `json:"l1_gas_price"`
L2GasPrice *ResourcePrice `json:"l2_gas_price"`
L2GasPrice *ResourcePrice `json:"l2_gas_price,omitempty"`
L1DataGasPrice *ResourcePrice `json:"l1_data_gas_price,omitempty"`
L1DAMode *L1DAMode `json:"l1_da_mode,omitempty"`
StarknetVersion string `json:"starknet_version"`
Expand Down Expand Up @@ -171,6 +171,15 @@
//
// It follows the specification defined here:
// https://github.com/starkware-libs/starknet-specs/blob/a789ccc3432c57777beceaa53a34a7ae2f25fda0/api/starknet_api_openrpc.json#L11
func (h *Handler) BlockWithTxHashesV0_7(id BlockID) (*BlockWithTxHashes, *jsonrpc.Error) {
blockWithTxHashes, rpcErr := h.BlockWithTxHashes(id)
if rpcErr != nil {
return nil, rpcErr
}
blockWithTxHashes.L2GasPrice = nil
return blockWithTxHashes, nil

Check warning on line 180 in rpc/block.go

View check run for this annotation

Codecov / codecov/patch

rpc/block.go#L174-L180

Added lines #L174 - L180 were not covered by tests
}

func (h *Handler) BlockWithTxHashes(id BlockID) (*BlockWithTxHashes, *jsonrpc.Error) {
block, rpcErr := h.blockByID(&id)
if rpcErr != nil {
Expand Down Expand Up @@ -207,7 +216,20 @@
return header.TransactionCount, nil
}

func (h *Handler) BlockWithReceiptsV0_7(id BlockID) (*BlockWithReceipts, *jsonrpc.Error) {
blockWithReceipts, err := h.blockWithReceipts(id, V0_7)
if err != nil {
return nil, err
}
blockWithReceipts.L2GasPrice = nil
return blockWithReceipts, nil

Check warning on line 225 in rpc/block.go

View check run for this annotation

Codecov / codecov/patch

rpc/block.go#L219-L225

Added lines #L219 - L225 were not covered by tests
}

func (h *Handler) BlockWithReceipts(id BlockID) (*BlockWithReceipts, *jsonrpc.Error) {
return h.blockWithReceipts(id, V0_8)
}

func (h *Handler) blockWithReceipts(id BlockID, rpcVersion version) (*BlockWithReceipts, *jsonrpc.Error) {
block, rpcErr := h.blockByID(&id)
if rpcErr != nil {
return nil, rpcErr
Expand All @@ -232,7 +254,7 @@
txsWithReceipts[index] = TransactionWithReceipt{
Transaction: t,
// block_hash, block_number are optional in BlockWithReceipts response
Receipt: AdaptReceipt(r, txn, finalityStatus, nil, 0),
Receipt: AdaptReceipt(r, txn, finalityStatus, nil, 0, rpcVersion),
}
}

Expand All @@ -247,6 +269,15 @@
//
// It follows the specification defined here:
// https://github.com/starkware-libs/starknet-specs/blob/a789ccc3432c57777beceaa53a34a7ae2f25fda0/api/starknet_api_openrpc.json#L44
func (h *Handler) BlockWithTxsV0_7(id BlockID) (*BlockWithTxs, *jsonrpc.Error) {
blockWithTxs, rpcErr := h.BlockWithTxs(id)
if rpcErr != nil {
return nil, rpcErr
}
blockWithTxs.L2GasPrice = nil
return blockWithTxs, nil

Check warning on line 278 in rpc/block.go

View check run for this annotation

Codecov / codecov/patch

rpc/block.go#L272-L278

Added lines #L272 - L278 were not covered by tests
}

func (h *Handler) BlockWithTxs(id BlockID) (*BlockWithTxs, *jsonrpc.Error) {
block, rpcErr := h.blockByID(&id)
if rpcErr != nil {
Expand Down
4 changes: 2 additions & 2 deletions rpc/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ func TestBlockWithReceipts(t *testing.T) {

txsWithReceipt = append(txsWithReceipt, rpc.TransactionWithReceipt{
Transaction: adaptedTx,
Receipt: rpc.AdaptReceipt(receipt, tx, rpc.TxnAcceptedOnL2, nil, 0),
Receipt: rpc.AdaptReceipt(receipt, tx, rpc.TxnAcceptedOnL2, nil, 0, rpc.V0_8),
})
}

Expand Down Expand Up @@ -678,7 +678,7 @@ func TestBlockWithReceipts(t *testing.T) {

transactions = append(transactions, rpc.TransactionWithReceipt{
Transaction: adaptedTx,
Receipt: rpc.AdaptReceipt(receipt, tx, rpc.TxnAcceptedOnL1, nil, 0),
Receipt: rpc.AdaptReceipt(receipt, tx, rpc.TxnAcceptedOnL1, nil, 0, rpc.V0_8),
})
}

Expand Down
76 changes: 39 additions & 37 deletions rpc/estimate_fee.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rpc

import (
"encoding/json"
"errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -28,15 +29,6 @@
}
}

type FeeEstimateV0_7 struct {
GasConsumed *felt.Felt `json:"gas_consumed"`
GasPrice *felt.Felt `json:"gas_price"`
DataGasConsumed *felt.Felt `json:"data_gas_consumed"`
DataGasPrice *felt.Felt `json:"data_gas_price"`
OverallFee *felt.Felt `json:"overall_fee"`
Unit *FeeUnit `json:"unit,omitempty"`
}

type FeeEstimate struct {
L1GasConsumed *felt.Felt `json:"l1_gas_consumed,omitempty"`
L1GasPrice *felt.Felt `json:"l1_gas_price,omitempty"`
Expand All @@ -46,40 +38,55 @@
L1DataGasPrice *felt.Felt `json:"l1_data_gas_price,omitempty"`
OverallFee *felt.Felt `json:"overall_fee"`
Unit *FeeUnit `json:"unit,omitempty"`
rpcVersion version
}

func (f FeeEstimate) MarshalJSON() ([]byte, error) {
switch f.rpcVersion {
case V0_7:
return json.Marshal(struct {
GasConsumed *felt.Felt `json:"gas_consumed"`
GasPrice *felt.Felt `json:"gas_price"`
DataGasConsumed *felt.Felt `json:"data_gas_consumed"`
DataGasPrice *felt.Felt `json:"data_gas_price"`
OverallFee *felt.Felt `json:"overall_fee"`
Unit *FeeUnit `json:"unit,omitempty"`
}{
GasConsumed: f.L1GasConsumed,
GasPrice: f.L1GasPrice,
DataGasConsumed: f.L1DataGasConsumed,
DataGasPrice: f.L1DataGasPrice,
OverallFee: f.OverallFee,
Unit: f.Unit,
})
case V0_8:
type alias FeeEstimate // avoid infinite recursion
return json.Marshal(alias(f))
default:
return nil, errors.New("unknown FeeEstimate version")

Check warning on line 66 in rpc/estimate_fee.go

View check run for this annotation

Codecov / codecov/patch

rpc/estimate_fee.go#L65-L66

Added lines #L65 - L66 were not covered by tests
}
}

/****************************************************
Estimate Fee Handlers
*****************************************************/

func feeEstimateToV0_7(feeEstimate FeeEstimate) FeeEstimateV0_7 {
return FeeEstimateV0_7{
GasConsumed: feeEstimate.L1GasConsumed,
GasPrice: feeEstimate.L1GasPrice,
DataGasConsumed: feeEstimate.L1DataGasConsumed,
DataGasPrice: feeEstimate.L1DataGasPrice,
OverallFee: feeEstimate.OverallFee,
Unit: feeEstimate.Unit,
}
}

func (h *Handler) EstimateFeeV0_7(broadcastedTxns []BroadcastedTransaction,
simulationFlags []SimulationFlag, id BlockID,
) ([]FeeEstimateV0_7, http.Header, *jsonrpc.Error) {
result, httpHeader, err := h.simulateTransactions(id, broadcastedTxns, append(simulationFlags, SkipFeeChargeFlag), true)
if err != nil {
return nil, httpHeader, err
}

return utils.Map(result, func(tx SimulatedTransaction) FeeEstimateV0_7 {
return feeEstimateToV0_7(tx.FeeEstimation)
}), httpHeader, nil
) ([]FeeEstimate, http.Header, *jsonrpc.Error) {
return h.estimateFee(broadcastedTxns, simulationFlags, id, V0_7)
}

func (h *Handler) EstimateFee(broadcastedTxns []BroadcastedTransaction,
simulationFlags []SimulationFlag, id BlockID,
) ([]FeeEstimate, http.Header, *jsonrpc.Error) {
result, httpHeader, err := h.simulateTransactions(id, broadcastedTxns, append(simulationFlags, SkipFeeChargeFlag), true)
return h.estimateFee(broadcastedTxns, simulationFlags, id, V0_8)
}

func (h *Handler) estimateFee(broadcastedTxns []BroadcastedTransaction,
simulationFlags []SimulationFlag, id BlockID, rpcVersion version,
) ([]FeeEstimate, http.Header, *jsonrpc.Error) {
result, httpHeader, err := h.simulateTransactions(id, broadcastedTxns, append(simulationFlags, SkipFeeChargeFlag), true, rpcVersion)
if err != nil {
return nil, httpHeader, err
}
Expand All @@ -90,13 +97,8 @@
}

//nolint:gocritic
func (h *Handler) EstimateMessageFeeV0_7(msg MsgFromL1, id BlockID) (*FeeEstimateV0_7, http.Header, *jsonrpc.Error) {
estimate, header, err := estimateMessageFee(msg, id, h.EstimateFee)
if err != nil {
return nil, header, err
}
estimateV0_7 := feeEstimateToV0_7(*estimate)
return &estimateV0_7, header, nil
func (h *Handler) EstimateMessageFeeV0_7(msg MsgFromL1, id BlockID) (*FeeEstimate, http.Header, *jsonrpc.Error) {
return estimateMessageFee(msg, id, h.EstimateFeeV0_7)

Check warning on line 101 in rpc/estimate_fee.go

View check run for this annotation

Codecov / codecov/patch

rpc/estimate_fee.go#L100-L101

Added lines #L100 - L101 were not covered by tests
}

//nolint:gocritic
Expand Down
78 changes: 45 additions & 33 deletions rpc/estimate_fee_pkg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,52 @@ import (
"testing"

"github.com/NethermindEth/juno/core/felt"
"github.com/stretchr/testify/assert"
"github.com/NethermindEth/juno/utils"
"github.com/stretchr/testify/require"
)

func TestFeeEstimateToV0_7(t *testing.T) {
t.Run("empty", func(t *testing.T) {
assert.Equal(t, FeeEstimateV0_7{}, feeEstimateToV0_7(FeeEstimate{}))
})

t.Run("full", func(t *testing.T) {
gasConsumed := new(felt.Felt).SetUint64(1)
gasPrice := new(felt.Felt).SetUint64(2)
dataGasConsumed := new(felt.Felt).SetUint64(3)
dataGasPrice := new(felt.Felt).SetUint64(4)
overallFee := new(felt.Felt).SetUint64(5)
unit := WEI
assert.Equal(
t,
FeeEstimateV0_7{
GasConsumed: gasConsumed,
GasPrice: gasPrice,
DataGasConsumed: dataGasConsumed,
DataGasPrice: dataGasPrice,
OverallFee: overallFee,
Unit: &unit,
func TestEstimateFeeMarshalJson(t *testing.T) {
tests := []struct {
name string
f FeeEstimate
want []byte
}{
{ //nolint:dupl
name: "V0_7",
f: FeeEstimate{
L1GasConsumed: new(felt.Felt).SetUint64(1),
L1GasPrice: new(felt.Felt).SetUint64(2),
L2GasConsumed: new(felt.Felt).SetUint64(3),
L2GasPrice: new(felt.Felt).SetUint64(4),
L1DataGasConsumed: new(felt.Felt).SetUint64(5),
L1DataGasPrice: new(felt.Felt).SetUint64(6),
OverallFee: new(felt.Felt).SetUint64(7),
Unit: utils.Ptr(WEI),
rpcVersion: V0_7,
},
want: []byte(`{"gas_consumed":"0x1","gas_price":"0x2","data_gas_consumed":"0x5","data_gas_price":"0x6","overall_fee":"0x7","unit":"WEI"}`),
},
{ //nolint:dupl
name: "V0_8",
f: FeeEstimate{
L1GasConsumed: new(felt.Felt).SetUint64(8),
L1GasPrice: new(felt.Felt).SetUint64(9),
L2GasConsumed: new(felt.Felt).SetUint64(10),
L2GasPrice: new(felt.Felt).SetUint64(11),
L1DataGasConsumed: new(felt.Felt).SetUint64(12),
L1DataGasPrice: new(felt.Felt).SetUint64(13),
OverallFee: new(felt.Felt).SetUint64(14),
Unit: utils.Ptr(WEI),
rpcVersion: V0_8,
},
feeEstimateToV0_7(FeeEstimate{
L1GasConsumed: gasConsumed,
L1GasPrice: gasPrice,
L2GasConsumed: new(felt.Felt).SetUint64(6),
L2GasPrice: new(felt.Felt).SetUint64(7),
L1DataGasConsumed: dataGasConsumed,
L1DataGasPrice: dataGasPrice,
OverallFee: overallFee,
Unit: &unit,
}))
})
want: []byte(`{"l1_gas_consumed":"0x8","l1_gas_price":"0x9","l2_gas_consumed":"0xa","l2_gas_price":"0xb","l1_data_gas_consumed":"0xc","l1_data_gas_price":"0xd","overall_fee":"0xe","unit":"WEI"}`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.f.MarshalJSON()
require.NoError(t, err)
require.Equal(t, tt.want, got)
})
}
}
25 changes: 19 additions & 6 deletions rpc/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"fmt"
"log"
"maps"
"math"
"strings"
stdsync "sync"
Expand Down Expand Up @@ -81,6 +82,13 @@ const (
throttledVMErr = "VM throughput limit reached"
)

type version uint32

const (
V0_7 version = iota + 1
V0_8
)

type traceCacheKey struct {
blockHash felt.Felt
}
Expand Down Expand Up @@ -195,7 +203,12 @@ func (h *Handler) Run(ctx context.Context) error {
feed.Tee(l1HeadsSub, h.l1Heads)

<-ctx.Done()
for _, sub := range h.subscriptions {

h.mu.Lock()
subscriptions := maps.Values(h.subscriptions)
h.mu.Unlock()

for sub := range subscriptions {
sub.wg.Wait()
}
return nil
Expand Down Expand Up @@ -414,12 +427,12 @@ func (h *Handler) MethodsV0_7() ([]jsonrpc.Method, string) { //nolint: funlen
{
Name: "starknet_getBlockWithTxHashes",
Params: []jsonrpc.Parameter{{Name: "block_id"}},
Handler: h.BlockWithTxHashes,
Handler: h.BlockWithTxHashesV0_7,
},
{
Name: "starknet_getBlockWithTxs",
Params: []jsonrpc.Parameter{{Name: "block_id"}},
Handler: h.BlockWithTxs,
Handler: h.BlockWithTxsV0_7,
},
{
Name: "starknet_getTransactionByHash",
Expand All @@ -429,7 +442,7 @@ func (h *Handler) MethodsV0_7() ([]jsonrpc.Method, string) { //nolint: funlen
{
Name: "starknet_getTransactionReceipt",
Params: []jsonrpc.Parameter{{Name: "transaction_hash"}},
Handler: h.TransactionReceiptByHash,
Handler: h.TransactionReceiptByHashV0_7,
},
{
Name: "starknet_getBlockTransactionCount",
Expand Down Expand Up @@ -537,7 +550,7 @@ func (h *Handler) MethodsV0_7() ([]jsonrpc.Method, string) { //nolint: funlen
{
Name: "starknet_simulateTransactions",
Params: []jsonrpc.Parameter{{Name: "block_id"}, {Name: "transactions"}, {Name: "simulation_flags"}},
Handler: h.SimulateTransactions,
Handler: h.SimulateTransactionsV0_7,
},
{
Name: "starknet_traceBlockTransactions",
Expand All @@ -561,7 +574,7 @@ func (h *Handler) MethodsV0_7() ([]jsonrpc.Method, string) { //nolint: funlen
{
Name: "starknet_getBlockWithReceipts",
Params: []jsonrpc.Parameter{{Name: "block_id"}},
Handler: h.BlockWithReceipts,
Handler: h.BlockWithReceiptsV0_7,
},
}, "/v0_7"
}
Loading