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

Add execution stepcount in header [X-Cairo-Steps] #1989

Merged
merged 29 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ca76d85
Update hashing functions for 0.13.2 (#1964)
kirugan Jul 30, 2024
752bf1e
Retrieve total execution steps for all transactions
obasekiosa Jul 31, 2024
ebf301a
Update vm execute interface to allow return of execution steps
obasekiosa Jul 31, 2024
f71f3d2
Fix typo in rust call for execution steps
obasekiosa Jul 31, 2024
f1f5242
Merge branch 'main' into feature/return-execution-stepcount-in-header
obasekiosa Jul 31, 2024
100f058
Fix: change execution steps type in c/g interface to ulonglong
obasekiosa Jul 31, 2024
191e19e
Fix: Update vm mock, fix trace tests and remove old comments.
obasekiosa Aug 1, 2024
ee32e62
Fix: Remove unnecessary comments
obasekiosa Aug 2, 2024
70e56fc
Fix: set default value in the case of error on rust-get execution ste…
obasekiosa Aug 2, 2024
657fa53
Update: change default value to u64::MAX on conversion usize to u64 f…
obasekiosa Aug 5, 2024
6d65ec7
Show: Just for show. This will fail test and this commit will be over…
obasekiosa Aug 6, 2024
ef54d71
Fix: complete handleRequest modification to allow headers
obasekiosa Aug 7, 2024
62e82af
Fix: temp handleBatchRequest modification to allow headers
obasekiosa Aug 7, 2024
3770005
Fix: lint errors
obasekiosa Aug 7, 2024
087bc24
Update: rename header variable
obasekiosa Aug 7, 2024
37b01d2
Merge branch 'main' into feature/return-execution-stepcount-in-header
obasekiosa Aug 7, 2024
08d1473
Update: combine headers for batch transactions
obasekiosa Aug 7, 2024
5d0a5f2
Update: refactor header combination
obasekiosa Aug 7, 2024
66ec3ff
Merge branch 'main' into feature/return-execution-stepcount-in-header
obasekiosa Aug 7, 2024
066c347
Revert: revert change in spec
obasekiosa Aug 8, 2024
e2f02ba
Fix: refactor and update tests
obasekiosa Aug 16, 2024
2bba179
Merge with main and fix vm related tests.
obasekiosa Aug 16, 2024
585c09f
Fix linter error
obasekiosa Aug 16, 2024
4df9731
Merge branch 'main' into feature/return-execution-stepcount-in-header
obasekiosa Aug 22, 2024
fb0c290
Update test to use variables for setup and expected values.
obasekiosa Aug 22, 2024
a777d52
Fix: fix tests due to previous merge from main. Update return values …
obasekiosa Aug 22, 2024
a729446
Update: remove unnecessary todo's
obasekiosa Aug 22, 2024
39ad69b
Tests simplification
kirugan Aug 23, 2024
7c8301d
Restructure assertions, fix formatting error
kirugan Aug 23, 2024
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
6 changes: 5 additions & 1 deletion jsonrpc/http.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package jsonrpc

import (
"maps"
"net/http"

"github.com/NethermindEth/juno/utils"
Expand Down Expand Up @@ -46,8 +47,11 @@ func (h *HTTP) ServeHTTP(writer http.ResponseWriter, req *http.Request) {

req.Body = http.MaxBytesReader(writer, req.Body, MaxRequestBodySize)
h.listener.OnNewRequest("any")
resp, err := h.rpc.HandleReader(req.Context(), req.Body)
resp, header, err := h.rpc.HandleReader(req.Context(), req.Body)

writer.Header().Set("Content-Type", "application/json")
maps.Copy(writer.Header(), header) // overwrites duplicate headers
rianhughes marked this conversation as resolved.
Show resolved Hide resolved

if err != nil {
h.log.Errorw("Handler failure", "err", err)
writer.WriteHeader(http.StatusInternalServerError)
Expand Down
90 changes: 64 additions & 26 deletions jsonrpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"errors"
"fmt"
"io"
"net/http"
"reflect"
"strings"
"sync"
Expand Down Expand Up @@ -183,11 +184,18 @@
if numArgs != len(method.Params) {
return errors.New("number of non-context function params and param names must match")
}
if handlerT.NumOut() != 2 {
return errors.New("handler must return 2 values")
outSize := handlerT.NumOut()
if outSize < 2 || outSize > 3 {
return errors.New("handler must return 2 or 3 values")
}
if handlerT.Out(1) != reflect.TypeOf(&Error{}) {
return errors.New("second return value must be a *jsonrpc.Error")
if outSize == 2 && handlerT.Out(1) != reflect.TypeOf(&Error{}) {
return errors.New("second return value must be a *jsonrpc.Error for 2 tuple handler")
} else if outSize == 3 && handlerT.Out(2) != reflect.TypeOf(&Error{}) {
return errors.New("third return value must be a *jsonrpc.Error for 3 tuple handler")
}

if outSize == 3 && handlerT.Out(1) != reflect.TypeOf(http.Header{}) {
return errors.New("second return value must be a http.Header for 3 tuple handler")
}

// The method is valid. Mutate the appropriate fields and register on the server.
Expand Down Expand Up @@ -255,7 +263,8 @@
activated: activated,
}
msgCtx := context.WithValue(ctx, ConnKey{}, conn)
resp, err := s.HandleReader(msgCtx, rw)
// header is unnecessary for read-writer(websocket)
resp, _, err := s.HandleReader(msgCtx, rw)
rianhughes marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
conn.initialErr = err
return err
Expand All @@ -272,27 +281,31 @@
// HandleReader processes a request to the server
// It returns the response in a byte array, only returns an
// error if it can not create the response byte array
func (s *Server) HandleReader(ctx context.Context, reader io.Reader) ([]byte, error) {
func (s *Server) HandleReader(ctx context.Context, reader io.Reader) ([]byte, http.Header, error) {
bufferedReader := bufio.NewReaderSize(reader, bufferSize)
requestIsBatch := isBatch(bufferedReader)
res := &response{
Version: "2.0",
}

header := http.Header{}

dec := json.NewDecoder(bufferedReader)
dec.UseNumber()

if !requestIsBatch {
req := new(Request)
if jsonErr := dec.Decode(req); jsonErr != nil {
res.Error = Err(InvalidJSON, jsonErr.Error())
} else if resObject, handleErr := s.handleRequest(ctx, req); handleErr != nil {
} else if resObject, httpHeader, handleErr := s.handleRequest(ctx, req); handleErr != nil {
if !errors.Is(handleErr, ErrInvalidID) {
res.ID = req.ID
}
res.Error = Err(InvalidRequest, handleErr.Error())
header = httpHeader
} else {
res = resObject
header = httpHeader
}
} else {
var batchReq []json.RawMessage
Expand All @@ -307,23 +320,27 @@
}

if res == nil {
return nil, nil
return nil, header, nil
}
return json.Marshal(res)

result, err := json.Marshal(res)
return result, header, err
}

func (s *Server) handleBatchRequest(ctx context.Context, batchReq []json.RawMessage) ([]byte, error) {
func (s *Server) handleBatchRequest(ctx context.Context, batchReq []json.RawMessage) ([]byte, http.Header, error) {
var (
responses []json.RawMessage
mutex sync.Mutex
responses []json.RawMessage
headers []http.Header
)

addResponse := func(response any) {
addResponse := func(response any, header http.Header) {
if responseJSON, err := json.Marshal(response); err != nil {
s.log.Errorw("failed to marshal response", "err", err)
} else {
mutex.Lock()
responses = append(responses, responseJSON)
headers = append(headers, header)
mutex.Unlock()
}
}
Expand All @@ -341,15 +358,15 @@
addResponse(&response{
Version: "2.0",
Error: Err(InvalidRequest, err.Error()),
})
}, http.Header{})
continue
}

wg.Add(1)
s.pool.Go(func() {
defer wg.Done()

resp, err := s.handleRequest(ctx, req)
resp, header, err := s.handleRequest(ctx, req)
if err != nil {
resp = &response{
Version: "2.0",
Expand All @@ -359,20 +376,33 @@
resp.ID = req.ID
}
}
// for notification request response is nil
// for notification request response is nil and header is irrelevant for now
if resp != nil {
addResponse(resp)
addResponse(resp, header)
}
})
}

wg.Wait()

// merge headers
finalHeaders := http.Header{}
for _, header := range headers {
for k, v := range header {
for _, e := range v {
finalHeaders.Add(k, e)

Check warning on line 393 in jsonrpc/server.go

View check run for this annotation

Codecov / codecov/patch

jsonrpc/server.go#L392-L393

Added lines #L392 - L393 were not covered by tests
}
}
}
kirugan marked this conversation as resolved.
Show resolved Hide resolved

// according to the spec if there are no response objects server must not return empty array
if len(responses) == 0 {
return nil, nil
return nil, finalHeaders, nil
}

return json.Marshal(responses)
result, err := json.Marshal(responses)

return result, finalHeaders, err // todo: fix batch request aggregate header
obasekiosa marked this conversation as resolved.
Show resolved Hide resolved
}

func isBatch(reader *bufio.Reader) bool {
Expand All @@ -396,11 +426,13 @@
return i == nil || reflect.ValueOf(i).IsNil()
}

func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, error) {
func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, http.Header, error) {
s.log.Tracew("Received request", "req", req)

header := http.Header{}
if err := req.isSane(); err != nil {
s.log.Tracew("Request sanity check failed", "err", err)
return nil, err
return nil, header, err
}

res := &response{
Expand All @@ -412,7 +444,7 @@
if !found {
res.Error = Err(MethodNotFound, nil)
s.log.Tracew("Method not found in request", "method", req.Method)
return res, nil
return res, header, nil
}

handlerTimer := time.Now()
Expand All @@ -421,7 +453,7 @@
if err != nil {
res.Error = Err(InvalidParams, err.Error())
s.log.Tracew("Error building arguments for RPC call", "err", err)
return res, nil
return res, header, nil
}
defer func() {
s.listener.OnRequestHandled(req.Method, time.Since(handlerTimer))
Expand All @@ -430,22 +462,28 @@
tuple := reflect.ValueOf(calledMethod.Handler).Call(args)
if res.ID == nil { // notification
s.log.Tracew("Notification received, no response expected")
return nil, nil
return nil, header, nil
}

errorIndex := 1
if len(tuple) == 3 {
errorIndex = 2
header = (tuple[1].Interface()).(http.Header)

Check warning on line 471 in jsonrpc/server.go

View check run for this annotation

Codecov / codecov/patch

jsonrpc/server.go#L470-L471

Added lines #L470 - L471 were not covered by tests
}

if errAny := tuple[1].Interface(); !isNil(errAny) {
if errAny := tuple[errorIndex].Interface(); !isNil(errAny) {
res.Error = errAny.(*Error)
if res.Error.Code == InternalError {
s.listener.OnRequestFailed(req.Method, res.Error)
reqJSON, _ := json.Marshal(req)
errJSON, _ := json.Marshal(res.Error)
s.log.Debugw("Failed handing RPC request", "req", string(reqJSON), "res", string(errJSON))
}
return res, nil
return res, header, nil
}
res.Result = tuple[0].Interface()

return res, nil
return res, header, nil
}

func (s *Server) buildArguments(ctx context.Context, params any, method Method) ([]reflect.Value, error) {
Expand Down
25 changes: 19 additions & 6 deletions jsonrpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,27 @@ func TestServer_RegisterMethod(t *testing.T) {
"no return": {
handler: func(param1, param2 int) {},
paramNames: []jsonrpc.Parameter{{Name: "param1"}, {Name: "param2"}},
want: "handler must return 2 values",
want: "handler must return 2 or 3 values",
},
"int return": {
handler: func(param1, param2 int) (int, int) { return 0, 0 },
paramNames: []jsonrpc.Parameter{{Name: "param1"}, {Name: "param2"}},
want: "second return value must be a *jsonrpc.Error",
want: "second return value must be a *jsonrpc.Error for 2 tuple handler",
},
"no error return 3": {
handler: func(param1, param2 int) (int, int, int) { return 0, 0, 0 },
paramNames: []jsonrpc.Parameter{{Name: "param1"}, {Name: "param2"}},
want: "third return value must be a *jsonrpc.Error for 3 tuple handler",
},
"no header return 3": {
handler: func(param1, param2 int) (int, int, *jsonrpc.Error) { return 0, 0, &jsonrpc.Error{} },
paramNames: []jsonrpc.Parameter{{Name: "param1"}, {Name: "param2"}},
want: "second return value must be a http.Header for 3 tuple handler",
},
"no error return": {
handler: func(param1, param2 int) (any, int) { return 0, 0 },
paramNames: []jsonrpc.Parameter{{Name: "param1"}, {Name: "param2"}},
want: "second return value must be a *jsonrpc.Error",
want: "second return value must be a *jsonrpc.Error for 2 tuple handler",
},
}

Expand Down Expand Up @@ -472,8 +482,9 @@ func TestHandle(t *testing.T) {
oldRequestFailedEventCount := len(listener.OnRequestFailedCalls)
oldRequestHandledCalls := len(listener.OnRequestHandledCalls)

res, err := server.HandleReader(context.Background(), strings.NewReader(test.req))
res, httpHeader, err := server.HandleReader(context.Background(), strings.NewReader(test.req))
require.NoError(t, err)
assert.NotNil(t, httpHeader)

if test.isBatch {
assertBatchResponse(t, test.res, string(res))
Expand Down Expand Up @@ -515,8 +526,9 @@ func BenchmarkHandle(b *testing.B) {

const request = `{"jsonrpc":"2.0","id":1,"method":"test"}`
for i := 0; i < b.N; i++ {
_, err := server.HandleReader(context.Background(), strings.NewReader(request))
_, header, err := server.HandleReader(context.Background(), strings.NewReader(request))
require.NoError(b, err)
require.NotNil(b, header)
}
}

Expand All @@ -531,9 +543,10 @@ func TestCannotWriteToConnInHandler(t *testing.T) {
return 0, nil
},
}))
res, err := server.HandleReader(context.Background(), strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"test"}`))
res, header, err := server.HandleReader(context.Background(), strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"test"}`))
require.NoError(t, err)
require.Equal(t, `{"jsonrpc":"2.0","result":0,"id":1}`, string(res))
require.NotNil(t, header)
}

type fakeConn struct{}
Expand Down
7 changes: 4 additions & 3 deletions mocks/mock_vm.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions node/throttled_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@
func (tvm *ThrottledVM) Execute(txns []core.Transaction, declaredClasses []core.Class, paidFeesOnL1 []*felt.Felt,
blockInfo *vm.BlockInfo, state core.StateReader, network *utils.Network, skipChargeFee, skipValidate, errOnRevert,
useBlobData bool,
) ([]*felt.Felt, []*felt.Felt, []vm.TransactionTrace, error) {
) ([]*felt.Felt, []*felt.Felt, []vm.TransactionTrace, uint64, error) {
var ret []*felt.Felt
var traces []vm.TransactionTrace
var dataGasConsumed []*felt.Felt
return ret, dataGasConsumed, traces, tvm.Do(func(vm *vm.VM) error {
var numSteps uint64
return ret, dataGasConsumed, traces, numSteps, tvm.Do(func(vm *vm.VM) error {
var err error
ret, dataGasConsumed, traces, err = (*vm).Execute(txns, declaredClasses, paidFeesOnL1, blockInfo, state, network,
ret, dataGasConsumed, traces, numSteps, err = (*vm).Execute(txns, declaredClasses, paidFeesOnL1, blockInfo, state, network,

Check warning on line 43 in node/throttled_vm.go

View check run for this annotation

Codecov / codecov/patch

node/throttled_vm.go#L43

Added line #L43 was not covered by tests
skipChargeFee, skipValidate, errOnRevert, useBlobData)
return err
})
Expand Down
Loading
Loading