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

ws: allow filtering notification by parameters #3689

Open
wants to merge 2 commits 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
12 changes: 9 additions & 3 deletions docs/notifications.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ Currently supported events:
Contents: transaction. Filters: sender and signer.
* notification generated during execution

Contents: container hash, contract hash, notification name, stack item. Filters: contract hash, notification name.
Contents: container hash, contract hash, notification name, stack item.
Filters: contract hash, notification name, notification parameters.
* transaction/persisting script executed

Contents: application execution result. Filters: VM state, script container hash.
Expand Down Expand Up @@ -84,9 +85,14 @@ Recognized stream names:
format for one of transaction's `Signers`.
* `notification_from_execution`
Filter: `contract` field containing a string with hex-encoded Uint160 (LE
representation) and/or `name` field containing a string with execution
representation), `name` field containing a string with execution
notification name which should be a valid UTF-8 string not longer than
32 bytes.
32 bytes and/or `parameters` field containing an ordered array of structs
with `type` and `value` fields. Parameter's `type` must be not-a-complex
type from the list: `Any`, `Boolean`, `Integer`, `ByteArray`, `String`,
`Hash160`, `Hash256`, `PublicKey` or `Signature`. Filter that allows any
parameter must be omitted or must be `Any` typed with zero value. It is
prohibited to have `parameters` be filled with `Any` types only.
* `transaction_executed`
Filter: `state` field containing `HALT` or `FAULT` string for successful
and failed executions respectively and/or `container` field containing
Expand Down
93 changes: 88 additions & 5 deletions pkg/neorpc/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,21 @@ package neorpc
import (
"errors"
"fmt"
"slices"

"github.com/nspcc-dev/neo-go/pkg/core/interop/runtime"
"github.com/nspcc-dev/neo-go/pkg/core/mempoolevent"
"github.com/nspcc-dev/neo-go/pkg/smartcontract"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/nspcc-dev/neo-go/pkg/vm/vmstate"
)

// MaxNotificationFilterParametersCount is a reasonable filter's parameter limit
// that does not allow attackers to increase node resources usage but that
// also should be enough for real applications.
const MaxNotificationFilterParametersCount = 16

type (
// BlockFilter is a wrapper structure for the block event filter. It allows
// to filter blocks by primary index and/or by block index (allowing blocks
Expand All @@ -29,11 +37,26 @@ type (
}
// NotificationFilter is a wrapper structure representing a filter used for
// notifications generated during transaction execution. Notifications can
// be filtered by contract hash and/or by name. nil value treated as missing
// filter.
// be filtered by contract hash, by event name and/or by notification
// parameters. Notification parameter filters will be applied in the order
// corresponding to a produced notification's parameters. `Any`-typed
// parameter with zero value allows any notification parameter. Supported
// parameter types:
// - [smartcontract.AnyType]
// - [smartcontract.BoolType]
// - [smartcontract.IntegerType]
// - [smartcontract.ByteArrayType]
// - [smartcontract.StringType]
// - [smartcontract.Hash160Type]
// - [smartcontract.Hash256Type]
// - [smartcontract.PublicKeyType]
// - [smartcontract.SignatureType]
// nil value treated as missing filter.
AnnaShaleva marked this conversation as resolved.
Show resolved Hide resolved
NotificationFilter struct {
Contract *util.Uint160 `json:"contract,omitempty"`
Name *string `json:"name,omitempty"`
Contract *util.Uint160 `json:"contract,omitempty"`
Name *string `json:"name,omitempty"`
Parameters []smartcontract.Parameter `json:"parameters,omitempty"`
parametersCache []stackitem.Item
}
// ExecutionFilter is a wrapper structure used for transaction and persisting
// scripts execution events. It allows to choose failing or successful
Expand Down Expand Up @@ -112,7 +135,10 @@ func (f TxFilter) IsValid() error {
return nil
}

// Copy creates a deep copy of the NotificationFilter. It handles nil NotificationFilter correctly.
// Copy creates a deep copy of the NotificationFilter. If
// [NotificationFilter.ParametersAsStackItems] has been called before,
// cached values are cleared. It handles nil NotificationFilter
// correctly.
func (f *NotificationFilter) Copy() *NotificationFilter {
if f == nil {
return nil
Expand All @@ -126,14 +152,71 @@ func (f *NotificationFilter) Copy() *NotificationFilter {
res.Name = new(string)
*res.Name = *f.Name
}
if len(f.Parameters) != 0 {
res.Parameters = slices.Clone(f.Parameters)
}
f.parametersCache = nil
Copy link
Member

Choose a reason for hiding this comment

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

But why? It's res.Parameters cache that should be cleaned (but it's not set anyway). So just remove f.parametersCache = nil and adjust method documentation a bit.

Copy link
Member Author

Choose a reason for hiding this comment

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

i tried to follow your suggestion, i believe:

Cache should be cleaned on filter's Copy.

i treat it like a new life for the struct (otherwise i do not know why somebody needs to copy something). if you copy smth, you can try to reuse the original struct and then it will be unexpected when you have copied the struct, changed f.Parameters, and then called ParametersSI with the old return values

return res
}

// ParametersAsStackItems returns [stackitem.Item] version of [NotificationFilter.Parameters]
// according to [smartcontract.Parameter.ToStackItem]; Notice that the result is cached
// internally in [NotificationFilter] for efficiency, so once you call this method it will
// not change even if you change any structure fields. If you need to update parameters, use
// [NotificationFilter.Copy].It mainly should be used by server code. Must not be used
// concurrently.
func (f *NotificationFilter) ParametersAsStackItems() ([]stackitem.Item, error) {
if len(f.Parameters) == 0 {
return nil, nil
}
if f.parametersCache != nil {
return f.parametersCache, nil
}
f.parametersCache = make([]stackitem.Item, 0, len(f.Parameters))
for i, p := range f.Parameters {
si, err := p.ToStackItem()
if err != nil {
f.parametersCache = nil
return nil, fmt.Errorf("converting %d parameter to stack item: %w", i, err)
}
f.parametersCache = append(f.parametersCache, si)
}
return f.parametersCache, nil
}

// IsValid implements SubscriptionFilter interface.
func (f NotificationFilter) IsValid() error {
if f.Name != nil && len(*f.Name) > runtime.MaxEventNameLen {
return fmt.Errorf("%w: NotificationFilter name parameter must be less than %d", ErrInvalidSubscriptionFilter, runtime.MaxEventNameLen)
}
noopFilter := true
if l := len(f.Parameters); l != 0 {
if l > MaxNotificationFilterParametersCount {
return fmt.Errorf("%w: NotificationFilter's parameters number exceeded: %d > %d", ErrInvalidSubscriptionFilter, l, MaxNotificationFilterParametersCount)
}
for i, parameter := range f.Parameters {
switch parameter.Type {
case smartcontract.BoolType,
smartcontract.IntegerType,
smartcontract.ByteArrayType,
smartcontract.StringType,
smartcontract.Hash160Type,
smartcontract.Hash256Type,
smartcontract.PublicKeyType,
smartcontract.SignatureType:
noopFilter = false
case smartcontract.AnyType:
default:
return fmt.Errorf("%w: NotificationFilter type parameter %d is unsupported: %s", ErrInvalidSubscriptionFilter, i, parameter.Type)
}
if _, err := parameter.ToStackItem(); err != nil {
return fmt.Errorf("%w: NotificationFilter %d filter parameter does not correspond to any stack item: %w", ErrInvalidSubscriptionFilter, i, err)
}
}
}
if noopFilter {
return fmt.Errorf("%w: NotificationFilter cannot have all parameters of type %s", ErrInvalidSubscriptionFilter, smartcontract.AnyType)
}
return nil
}

Expand Down
10 changes: 10 additions & 0 deletions pkg/neorpc/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package neorpc
import (
"testing"

"github.com/nspcc-dev/neo-go/pkg/smartcontract"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -88,6 +89,15 @@ func TestNotificationFilterCopy(t *testing.T) {
require.Equal(t, bf, tf)
*bf.Name = "azaza"
require.NotEqual(t, bf, tf)

var err error
bf.Parameters, err = smartcontract.NewParametersFromValues(1, "2", []byte{3})
require.NoError(t, err)

tf = bf.Copy()
require.Equal(t, bf, tf)
bf.Parameters[0], bf.Parameters[1] = bf.Parameters[1], bf.Parameters[0]
require.NotEqual(t, bf, tf)
}

func TestExecutionFilterCopy(t *testing.T) {
Expand Down
23 changes: 22 additions & 1 deletion pkg/neorpc/rpcevent/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/nspcc-dev/neo-go/pkg/core/transaction"
"github.com/nspcc-dev/neo-go/pkg/neorpc"
"github.com/nspcc-dev/neo-go/pkg/neorpc/result"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
)

type (
Expand Down Expand Up @@ -66,7 +67,27 @@ func Matches(f Comparator, r Container) bool {
notification := r.EventPayload().(*state.ContainedNotificationEvent)
hashOk := filt.Contract == nil || notification.ScriptHash.Equals(*filt.Contract)
nameOk := filt.Name == nil || notification.Name == *filt.Name
return hashOk && nameOk
parametersOk := true
if len(filt.Parameters) > 0 {
AnnaShaleva marked this conversation as resolved.
Show resolved Hide resolved
stackItems := notification.Item.Value().([]stackitem.Item)
AnnaShaleva marked this conversation as resolved.
Show resolved Hide resolved
parameters, err := filt.ParametersAsStackItems()
if err != nil {
return false
}
if len(parameters) > len(stackItems) {
return false
}
for i, p := range parameters {
if p.Type() == stackitem.AnyT && p.Value() == nil {
continue
}
if p.Type() != stackItems[i].Type() || !p.Equals(stackItems[i]) {
Copy link
Member

Choose a reason for hiding this comment

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

Remove type check, Equals will handle it.

Copy link
Member Author

Choose a reason for hiding this comment

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

well, i am lost then or do not understand smth. i have added it to follow your #3689 (comment). it was not here originally

parametersOk = false
break
}
}
}
return hashOk && nameOk && parametersOk
case neorpc.ExecutionEventID:
filt := filter.(neorpc.ExecutionFilter)
applog := r.EventPayload().(*state.AppExecResult)
Expand Down
44 changes: 44 additions & 0 deletions pkg/neorpc/rpcevent/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"github.com/nspcc-dev/neo-go/pkg/neorpc"
"github.com/nspcc-dev/neo-go/pkg/neorpc/result"
"github.com/nspcc-dev/neo-go/pkg/network/payload"
"github.com/nspcc-dev/neo-go/pkg/smartcontract"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/nspcc-dev/neo-go/pkg/vm/vmstate"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -55,6 +57,10 @@ func TestMatches(t *testing.T) {
name := "ntf name"
badName := "bad name"
badType := mempoolevent.TransactionRemoved
parameters, err := smartcontract.NewParametersFromValues(1, "2", []byte{3})
require.NoError(t, err)
badParameters, err := smartcontract.NewParametersFromValues([]byte{3}, "2", []byte{1})
require.NoError(t, err)
bContainer := testContainer{
id: neorpc.BlockEventID,
pld: &block.Block{
Expand All @@ -76,6 +82,16 @@ func TestMatches(t *testing.T) {
id: neorpc.NotificationEventID,
pld: &state.ContainedNotificationEvent{NotificationEvent: state.NotificationEvent{ScriptHash: contract, Name: name}},
}
ntfContainerParameters := testContainer{
id: neorpc.NotificationEventID,
pld: &state.ContainedNotificationEvent{
NotificationEvent: state.NotificationEvent{
ScriptHash: contract,
Name: name,
Item: stackitem.NewArray(prmsToStack(t, parameters)),
},
},
}
exContainer := testContainer{
id: neorpc.ExecutionEventID,
pld: &state.AppExecResult{Container: cnt, Execution: state.Execution{VMState: st}},
Expand Down Expand Up @@ -261,6 +277,24 @@ func TestMatches(t *testing.T) {
container: ntfContainer,
expected: true,
},
{
name: "notification, parameters match",
comparator: testComparator{
id: neorpc.NotificationEventID,
filter: neorpc.NotificationFilter{Name: &name, Contract: &contract, Parameters: parameters},
},
container: ntfContainerParameters,
expected: true,
},
{
name: "notification, parameters mismatch",
comparator: testComparator{
id: neorpc.NotificationEventID,
filter: neorpc.NotificationFilter{Name: &name, Contract: &contract, Parameters: badParameters},
},
container: ntfContainerParameters,
expected: false,
},
{
name: "execution, no filter",
comparator: testComparator{id: neorpc.ExecutionEventID},
Expand Down Expand Up @@ -343,3 +377,13 @@ func TestMatches(t *testing.T) {
})
}
}

func prmsToStack(t *testing.T, pp []smartcontract.Parameter) []stackitem.Item {
res := make([]stackitem.Item, 0, len(pp))
for _, p := range pp {
s, err := p.ToStackItem()
require.NoError(t, err)
res = append(res, s)
}
return res
}
24 changes: 24 additions & 0 deletions pkg/rpcclient/wsclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/nspcc-dev/neo-go/pkg/neorpc/result"
"github.com/nspcc-dev/neo-go/pkg/network/payload"
"github.com/nspcc-dev/neo-go/pkg/services/rpcsrv/params"
"github.com/nspcc-dev/neo-go/pkg/smartcontract"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/vmstate"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -591,6 +592,7 @@ func TestWSFilteredSubscriptions(t *testing.T) {
require.NoError(t, json.Unmarshal(param.RawMessage, filt))
require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Contract)
require.Nil(t, filt.Name)
require.Empty(t, filt.Parameters)
},
},
{"notifications name",
Expand All @@ -605,6 +607,7 @@ func TestWSFilteredSubscriptions(t *testing.T) {
require.NoError(t, json.Unmarshal(param.RawMessage, filt))
require.Equal(t, "my_pretty_notification", *filt.Name)
require.Nil(t, filt.Contract)
require.Empty(t, filt.Parameters)
},
},
{"notifications contract hash and name",
Expand All @@ -620,6 +623,27 @@ func TestWSFilteredSubscriptions(t *testing.T) {
require.NoError(t, json.Unmarshal(param.RawMessage, filt))
require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Contract)
require.Equal(t, "my_pretty_notification", *filt.Name)
require.Empty(t, filt.Parameters)
},
},
{"notifications parameters",
func(t *testing.T, wsc *WSClient) {
contract := util.Uint160{1, 2, 3, 4, 5}
name := "my_pretty_notification"
prms, err := smartcontract.NewParametersFromValues(1, "2", []byte{3})
require.NoError(t, err)
_, err = wsc.ReceiveExecutionNotifications(&neorpc.NotificationFilter{Contract: &contract, Name: &name, Parameters: prms}, make(chan *state.ContainedNotificationEvent))
require.NoError(t, err)
},
func(t *testing.T, p *params.Params) {
param := p.Value(1)
filt := new(neorpc.NotificationFilter)
prms, err := smartcontract.NewParametersFromValues(1, "2", []byte{3})
require.NoError(t, err)
require.NoError(t, json.Unmarshal(param.RawMessage, filt))
require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Contract)
require.Equal(t, "my_pretty_notification", *filt.Name)
require.Equal(t, prms, filt.Parameters)
},
},
{"executions state",
Expand Down
10 changes: 5 additions & 5 deletions pkg/services/rpcsrv/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestClient_NEP17(t *testing.T) {
t.Cleanup(c.Close)
require.NoError(t, c.Init())

h, err := util.Uint160DecodeStringLE(testContractHash)
h, err := util.Uint160DecodeStringLE(testContractHashLE)
require.NoError(t, err)
rub := nep17.NewReader(invoker.New(c, nil), h)

Expand Down Expand Up @@ -295,7 +295,7 @@ func TestClientManagementContract(t *testing.T) {
ids, err := manReader.GetContractHashesExpanded(10)
require.NoError(t, err)
ctrs := make([]management.IDHash, 0)
for i, s := range []string{testContractHash, verifyContractHash, verifyWithArgsContractHash, nnsContractHash, nfsoContractHash, storageContractHash} {
for i, s := range []string{testContractHashLE, verifyContractHash, verifyWithArgsContractHash, nnsContractHash, nfsoContractHash, storageContractHash} {
h, err := util.Uint160DecodeStringLE(s)
require.NoError(t, err)
ctrs = append(ctrs, management.IDHash{ID: int32(i) + 1, Hash: h})
Expand Down Expand Up @@ -2288,7 +2288,7 @@ func TestClient_FindStorage(t *testing.T) {
t.Cleanup(c.Close)
require.NoError(t, c.Init())

h, err := util.Uint160DecodeStringLE(testContractHash)
h, err := util.Uint160DecodeStringLE(testContractHashLE)
require.NoError(t, err)
prefix := []byte("aa")
expected := result.FindStorage{
Expand Down Expand Up @@ -2355,7 +2355,7 @@ func TestClient_FindStorageHistoric(t *testing.T) {

root, err := util.Uint256DecodeStringLE(block20StateRootLE)
require.NoError(t, err)
h, err := util.Uint160DecodeStringLE(testContractHash)
h, err := util.Uint160DecodeStringLE(testContractHashLE)
require.NoError(t, err)
prefix := []byte("aa")
expected := result.FindStorage{
Expand Down Expand Up @@ -2424,7 +2424,7 @@ func TestClient_GetStorageHistoric(t *testing.T) {

root, err := util.Uint256DecodeStringLE(block20StateRootLE)
require.NoError(t, err)
h, err := util.Uint160DecodeStringLE(testContractHash)
h, err := util.Uint160DecodeStringLE(testContractHashLE)
require.NoError(t, err)
key := []byte("aa10")
expected := []byte("v2")
Expand Down
Loading
Loading