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

Get block notifications API #3781

Open
wants to merge 1 commit into
base: master
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
101 changes: 101 additions & 0 deletions pkg/services/rpcsrv/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import (
"github.com/nspcc-dev/neo-go/pkg/vm/emit"
"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/nspcc-dev/neo-go/pkg/vm/vmstate"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -219,6 +220,7 @@ var rpcHandlers = map[string]func(*Server, params.Params) (any, *neorpc.Error){
"getblockhash": (*Server).getBlockHash,
"getblockheader": (*Server).getBlockHeader,
"getblockheadercount": (*Server).getBlockHeaderCount,
"getblocknotifications": (*Server).getBlockNotifications,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extension should be documented at docs/rpc.md, see ### Extensions section.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extension should be supported at the RPC client side (pkg/rpcclient/rpc.go).

"getblocksysfee": (*Server).getBlockSysFee,
"getcandidates": (*Server).getCandidates,
"getcommittee": (*Server).getCommittee,
Expand Down Expand Up @@ -3202,3 +3204,102 @@ func (s *Server) getRawNotaryTransaction(reqParams params.Params) (any, *neorpc.
}
return tx.Bytes(), nil
}

// getBlockNotifications returns notifications from a specific block with optional filtering.
func (s *Server) getBlockNotifications(reqParams params.Params) (any, *neorpc.Error) {
param := reqParams.Value(0)
hash, respErr := s.blockHashFromParam(param)
if respErr != nil {
return nil, respErr
}

block, err := s.chain.GetBlock(hash)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need some GetTrimmedBlock or alike, this one does extract all transactions and we don't need them (only hash is needed for GetAppExecResults()). But it's an optimization in its essence, current code will work fine.

if err != nil {
return nil, neorpc.ErrUnknownBlock
}

var filter *neorpc.NotificationFilter
if len(reqParams) > 1 {
filter = new(neorpc.NotificationFilter)
err := json.Unmarshal(reqParams[1].RawMessage, filter)
if err != nil {
return nil, neorpc.WrapErrorWithData(neorpc.ErrInvalidParams, fmt.Sprintf("invalid filter: %s", err))
}
if err := filter.IsValid(); err != nil {
return nil, neorpc.WrapErrorWithData(neorpc.ErrInvalidParams, fmt.Sprintf("invalid filter: %s", err))
}
}

var notifications []state.ContainedNotificationEvent
for _, tx := range block.Transactions {
aers, err := s.chain.GetAppExecResults(tx.Hash(), trigger.Application)
if err != nil {
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return internal error? This means some DB inconsistency and the result can be incorrect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vote up for error return.

}
for _, aer := range aers {
if aer.VMState == vmstate.Halt {
for _, evt := range aer.Events {
ntf := state.ContainedNotificationEvent{
Container: aer.Container,
NotificationEvent: evt,
}
if filter == nil || rpcevent.Matches(&testComparator{
id: neorpc.NotificationEventID,
filter: *filter,
}, &testContainer{ntf: &ntf}) {
notifications = append(notifications, ntf)
}
}
}
}
}

// Also check for notifications from the block itself (PostPersist)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to keep the sequence correct and check for trigger.OnPersist before digging into transaction list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may additionally extend neorpc.NotificaitonFilter with trigger parameter, it may be useful in some cases, what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the use cases? People interested in block-level things (and not a lot happens there) probably pull block applogs directly and otherwise it's all Application.

aers, err := s.chain.GetAppExecResults(block.Hash(), trigger.Application)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trigger.PostPersist here.

if err == nil && len(aers) > 0 {
aer := aers[0]
if aer.VMState == vmstate.Halt {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this code can be generalized in some way (helper function?), it's the same for blocks/transactions.

for _, evt := range aer.Events {
ntf := state.ContainedNotificationEvent{
Container: aer.Container,
NotificationEvent: evt,
}
if filter == nil || rpcevent.Matches(&testComparator{
id: neorpc.NotificationEventID,
filter: *filter,
}, &testContainer{ntf: &ntf}) {
notifications = append(notifications, ntf)
}
}
}
}

return notifications, nil
}

// testComparator is a helper type for notification filtering
type testComparator struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can move these out into a separate file (along with the logic for appending to notifications.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'd say that testComparator/testContainer is suitable for testing code, but not for the production code. Let's rename it to something more appropriate.

id neorpc.EventID
filter neorpc.NotificationFilter
}

func (t *testComparator) EventID() neorpc.EventID {
return t.id
}

func (t *testComparator) Filter() neorpc.SubscriptionFilter {
return t.filter
}

// testContainer is a helper type for notification filtering
type testContainer struct {
ntf *state.ContainedNotificationEvent
}

func (n *testContainer) EventID() neorpc.EventID {
return neorpc.NotificationEventID
}

func (n *testContainer) EventPayload() any {
return n.ntf
}
37 changes: 37 additions & 0 deletions pkg/services/rpcsrv/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2274,6 +2274,43 @@ var rpcTestCases = map[string][]rpcTestCase{
errCode: neorpc.InvalidParamsCode,
},
},
"getblocknotifications": {
{
name: "positive",
params: `["` + genesisBlockHash + `"]`,
result: func(e *executor) any { return []state.ContainedNotificationEvent{} },
check: func(t *testing.T, e *executor, acc any) {
res, ok := acc.([]state.ContainedNotificationEvent)
require.True(t, ok)
require.NotNil(t, res)
},
},
{
name: "positive with filter",
params: `["` + genesisBlockHash + `", {"contract":"` + testContractHashLE + `", "name":"Transfer"}]`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Genesis block doesn't contain notifications from testContract. Let's use native Gas or Neo hash as a filter.

result: func(e *executor) any { return []state.ContainedNotificationEvent{} },
check: func(t *testing.T, e *executor, acc any) {
res, ok := acc.([]state.ContainedNotificationEvent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check for expected contents as well.

require.True(t, ok)
require.NotNil(t, res)
},
},
{
name: "invalid hash",
params: `["invalid"]`,
fail: true,
},
{
name: "unknown block",
params: `["` + util.Uint256{}.StringLE() + `"]`,
fail: true,
},
{
name: "invalid filter",
params: `["` + genesisBlockHash + `", {"invalid":"filter"}]`,
fail: true,
},
},
}

func TestRPC(t *testing.T) {
Expand Down
Loading