Skip to content

Feat/meta data native contract#3759

Open
carpawell wants to merge 1 commit intomasterfrom
feat/meta-data-native-contract
Open

Feat/meta data native contract#3759
carpawell wants to merge 1 commit intomasterfrom
feat/meta-data-native-contract

Conversation

@carpawell
Copy link
Copy Markdown
Member

Contract part of #3742.

Copy link
Copy Markdown
Contributor

@cthulhu-rider cthulhu-rider left a comment

Choose a reason for hiding this comment

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

rebase is needed

panic(fmt.Sprintf("unexpected native contract found: %T", contract))
}
}
return append(newContracts, meta.NewMetadata(neoContract))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

meta.NewMetadata does not seem resistant to nil arg. I'd precheck it

in general, looking at for-loop, we dont check absence of any contract. If they are required, i'd still prevent this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added required contracts checks to NewCustomNatives

newContracts = append(newContracts, contract)
case *native.Std, *native.Crypto, *native.Oracle, *native.Treasury:
default:
panic(fmt.Sprintf("unexpected native contract found: %T", contract))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this done to not miss new contract?

since native.NewDefaultContracts does not state there will be no more contract, i'd say this is an error

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A new contract is a significant (and very rare) event, this forces explicit handling of it. Should be ok.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

is this done to not miss new contract?

yes, *native.Treasury was even added after this feature was started to be implemented. my idea was that once there is an update in neo-go, tests will start panicking, and we will immediately find out whether we need to adopt a new contract to our side chain or not

g := &GAS{
initialSupply: init,
}
defer g.BuildHFSpecificMD(g.ActiveIn())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

defer seems redundant

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is the way every native contract is implemented in neo-go, so i suggest to keep it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keep it.

return nil
}

// InitializeCache implements the Contract interface.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// InitializeCache implements the Contract interface.
// InitializeCache implements [native.IGAS] interface.

for all methods

}

// makeUint160Key creates a key from the account script hash.
func makeUint160Key(prefix byte, h util.Uint160) []byte {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unused too? Seems like some stuff is copy-pasted but not needed

return fmt.Errorf("%d vector has incorrect REP: %w", i, err)
}
rep := repB.Int64()
if rep > maxREPsClauses {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'd also check negatives

Nodes keys.PublicKeys
}

func (p Placement) ToStackItem() (stackitem.Item, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

used anywhere?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

for some reason lost resolution for this thread: #3742 (comment)

is used now

}
metaInfo, ok := metaInfoSI.Value().([]stackitem.MapElement)
if !ok {
panic(fmt.Errorf("unexpected deserialized meta information value: expected %T, %T, given", metaInfo, metaInfoSI.Value()))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
panic(fmt.Errorf("unexpected deserialized meta information value: expected %T, %T, given", metaInfo, metaInfoSI.Value()))
panic(fmt.Errorf("unexpected deserialized meta information value: expected %T, %T given", metaInfo, metaInfoSI.Value()))

}

if foundSigs == rep {
continue nodesLoop
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instruction seems excessive here, condition may be inverted

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

reimplementation of the byte-contract code from the neofs-contracts problems. fixed

panic("incorrect vub")
}
if vub.Int64() <= int64(ic.BlockHeight()) {
panic("incorrect vub: exceeded")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

runtime values may be helpful in exception

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added values everywhere a value is parsed and can be stringified

newContracts = append(newContracts, contract)
case *native.Std, *native.Crypto, *native.Oracle, *native.Treasury:
default:
panic(fmt.Sprintf("unexpected native contract found: %T", contract))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A new contract is a significant (and very rare) event, this forces explicit handling of it. Should be ok.

)

// nep17TokenNative represents a NEP-17 token contract.
type nep17TokenNative struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow. This should be imported from NeoGo native, really.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

really sorry, but it is not now

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be imported from NeoGo native

I don't think so. NeoGo's nep17TokenNative contains a lot of code that is specific to native Neo/Gas implementation and that is not needed for the simple meta GAS contract. I'd say that meta GAS doesn't even need nep17TokenNative, just define all methods on the GAS itself.

m.AddMethod(md, desc)

desc = native.NewDescriptor("registerMetaContainer", smartcontract.VoidType,
manifest.NewParameter("cID", smartcontract.Hash256Type))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

container

m.AddMethod(md, desc)

desc = native.NewDescriptor("unregisterMetaContainer", smartcontract.VoidType,
manifest.NewParameter("cID", smartcontract.Hash256Type))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here as well.

panic("container does not support chained metadata")
}

sigsVectorsRaw, ok := args[1].Value().([]stackitem.Item)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This follows current container contract, but eventually it can be removed from parameters, nodes can sign transaction directly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this will require responding there with a signature not of an object's metadata, but with a signature of some predefined transaction. is that what you meant?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes. Another option is a custom verification callback.

}
}

// required
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why cid is checked separately?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it is used in other checks above (container must exist and be registered as one that supports meta)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd rather have similar checks at the same place.


for _, sig := range sigVectors[i] {
for _, node := range placement[i].Nodes {
if node.Verify(sig, metaHash) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Slow.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i only suggest sorting signatures by public keys. requires support on the IR side and on the SN side

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not just about sorting. Remember, currently signatures are verified in verify() method of Proxy contract. This scales well. Checking them during transaction execution doesn't.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But sorting is beneficial anyway and easily done.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@roman-khimov, but what if number of nodes is bigger than 16? i found it occasionally when increased number of nodes in tests

do you mean that tx should be signed by nodes? or do you mean that we need Verify() method of the new native contract that should still get signatures from args and verify them (and, i guess, that new transactions should require the contract's signature to call Verify)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Multiple signatures for a single signer.

Copy link
Copy Markdown
Member Author

@carpawell carpawell Jan 20, 2026

Choose a reason for hiding this comment

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

@roman-khimov, did some multisignature magic, check, please, if i got you right

}
if v, ok := getFromMap(metaInfo, "firstPart"); ok {
firstPart, err := v.TryBytes()
if err != nil || len(firstPart) != smartcontract.Hash256Len {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sha256 extraction is pretty popular

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok-ok

if err != nil {
panic(err)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's it, no processing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

do you mean the secondary index must be in the same chain? my first implementation only needs side chain to check metadata and then notify about it. all the storage nodes handle notification on their own

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Among things current structure tries to solve is state synchronization. There is everything needed for it in blockchain, but if you're just throwing an event it all doesn't work. Primary index (based on submitted data) must be built directly on-chain. Then seconday one (header-based) is a different story.

Copy link
Copy Markdown
Member Author

@carpawell carpawell Jan 20, 2026

Choose a reason for hiding this comment

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

@roman-khimov, ok, adopted the previous metaservice approach. BUT currently there is oID+cID==64 problem for storage keys. i think we will eventually find a "one chain per container" solution, and it will be easier to work with keys, but currently i can only suggest skipping a single byte from oID

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you care much about key size limits in native code?

@carpawell carpawell force-pushed the feat/meta-data-native-contract branch 2 times, most recently from bd68082 to c8ae0ca Compare December 30, 2025 17:31
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 30, 2025

Codecov Report

❌ Patch coverage is 0% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.70%. Comparing base (c5f72db) to head (1f11e69).

Files with missing lines Patch % Lines
pkg/core/metachain/contracts.go 0.00% 27 Missing ⚠️
pkg/innerring/internal/blockchain/blockchain.go 0.00% 2 Missing ⚠️
pkg/innerring/internal/metachain/chain.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3759      +/-   ##
==========================================
- Coverage   25.73%   25.70%   -0.03%     
==========================================
  Files         670      672       +2     
  Lines       43062    43091      +29     
==========================================
- Hits        11081    11078       -3     
- Misses      30969    30998      +29     
- Partials     1012     1015       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@carpawell carpawell force-pushed the feat/meta-data-native-contract branch from c8ae0ca to 1f840ee Compare December 30, 2025 17:36
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch 2 times, most recently from b7a798d to 13c1f32 Compare January 19, 2026 07:19
@carpawell carpawell marked this pull request as draft January 19, 2026 08:09
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch 2 times, most recently from 978b2b0 to 4844514 Compare January 20, 2026 17:47
Copy link
Copy Markdown
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Better, but still can be optimized.

g := &GAS{
initialSupply: init,
}
defer g.BuildHFSpecificMD(g.ActiveIn())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keep it.

)

// nep17TokenNative represents a NEP-17 token contract.
type nep17TokenNative struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be imported from NeoGo native

I don't think so. NeoGo's nep17TokenNative contains a lot of code that is specific to native Neo/Gas implementation and that is not needed for the simple meta GAS contract. I'd say that meta GAS doesn't even need nep17TokenNative, just define all methods on the GAS itself.

Comment on lines +135 to +142
metaInfoSI, err := stackitem.Deserialize(metaInfoRaw)
if err != nil {
panic(fmt.Errorf("cannot deserialize meta information from byte array: %w", err))
}
metaInfo, ok := metaInfoSI.Value().([]stackitem.MapElement)
if !ok {
panic(fmt.Errorf("unexpected deserialized meta information value: expected %T, %T given", metaInfo, metaInfoSI.Value()))
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any usual contract that interacts with serialized metaInfo map? If not, then we may change the serialization format to bytes or raw JSON (like it's done for native ContractManagement's NEF/Manifest arguments of deploy/update), no need to keep stackitem.Map.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this adds overhead. @roman-khimov, thoughts?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Native Neo format has lower overhead compared to JSON and I don't see any immediate benefits of JSON here (for manifests that's rather easy, there are a lot of tools that are interested in manifests).


// NewCustomNatives returns custom list of native contracts for metadata
// side chain. Returned contracts:
// - NEO
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@roman-khimov, we don't need default NeoContract for this chain. We don't need voting and rewards at all, is it true?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i think, these lines require me to use smth, i tried to drop NEO contract. changing NEO to smth no-op can be done, i went the easiest way

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

do you mean to fill it with smth different just to cut out voting and all?

Copy link
Copy Markdown
Member

@AnnaShaleva AnnaShaleva Jan 23, 2026

Choose a reason for hiding this comment

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

You need to implement custom Neo contract (like you did it with custom Gas). This contract should implement minimal required functionality for meta chain: committee tracking and committee witness checking, that's it (although I'm not sure whether we need dynamic committee at meta chain; let's start with static set of keys). The rest of INEO functionality should be stubbed. Voting should be dropped.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You need to implement custom Neo contract

why i need it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For committee management.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently, we have decided to use regular Neo native since its excess functionality does not bother us for now, in the future, we may add a new minimal Neo contract

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Issue, please.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

where?

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Jan 23, 2026
Required for nspcc-dev/neofs-node#3759.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Jan 23, 2026
Required for nspcc-dev/neofs-node#3759.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Jan 26, 2026
Required for initial version of Meta native contracts constructor, ref.
nspcc-dev/neofs-node#3759.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch from 00d5d10 to 6eee48f Compare February 2, 2026 07:29
@carpawell carpawell marked this pull request as ready for review February 2, 2026 07:55
@carpawell
Copy link
Copy Markdown
Member Author

Pushed smth wrong...

@carpawell carpawell marked this pull request as draft February 2, 2026 07:55
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch from 6eee48f to be48655 Compare February 2, 2026 18:10
@carpawell carpawell marked this pull request as ready for review February 2, 2026 18:11
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch from be48655 to 35c50ed Compare February 2, 2026 18:12
@carpawell
Copy link
Copy Markdown
Member Author

Fixed.

Copy link
Copy Markdown
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Specific indexes can be tuned, but signatures seem to be ok. Let's test it.

md = native.NewMethodAndPrice(m.updateContainerList, 1<<15, callflag.WriteStates)
m.AddMethod(md, desc)

eDesc := native.NewEventDescriptor(putObjectEvent,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still a single event.

}

key[0] = addrIndex
deleteStorageItem(ic, key)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can I delete a lock?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hm, tbh, i do not know whether it should be double checked on the chain side. if every SN agrees that this object should be PUT, should a chain reject it?


for i, vector := range placement {
var foundSigs, lastFoundSig int
for _, sig := range sigVectors[i] {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is an optimized multisig checker in NeoGo, but likely not easily applicable.

@carpawell carpawell force-pushed the feat/meta-data-native-contract branch 2 times, most recently from 5e26b1d to 7648f14 Compare February 11, 2026 20:01
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch 2 times, most recently from 29d72e2 to 2ecbf8a Compare March 5, 2026 17:25
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch from 2ecbf8a to 36316ab Compare March 5, 2026 22:43
Also, implement new MetaData contract and GAS contract implementation without
economic.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell carpawell force-pushed the feat/meta-data-native-contract branch from 36316ab to 1f11e69 Compare March 6, 2026 17:08
g.decimals = 8

desc := native.NewDescriptor("symbol", smartcontract.StringType)
md := native.NewMethodAndPrice(g.Symbol, 0, callflag.NoneFlag)
Copy link
Copy Markdown
Member

@AnnaShaleva AnnaShaleva Mar 23, 2026

Choose a reason for hiding this comment

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

g.Symbol, g.Decimals and g.TotalSupply methods should be private.

desc = native.NewDescriptor("transfer", smartcontract.BoolType,
append(transferParams, manifest.NewParameter("data", smartcontract.AnyType))...,
)
md = native.NewMethodAndPrice(g.Transfer, 1<<17, callflag.States|callflag.AllowCall|callflag.AllowNotify)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Likely ditto for this one.


// BalanceOf returns native GAS token balance for the acc.
func (g *GAS) BalanceOf(d *dao.Simple, acc util.Uint160) *big.Int {
return big.NewInt(DefaultBalance * native.GASFactor)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
return big.NewInt(DefaultBalance * native.GASFactor)
return big.NewInt(DefaultBalance)

Because now DefaultBalance is 100 * native.GASFactor.

Comment on lines +33 to +37
type GAS struct {
interop.ContractMD
symbol string
decimals int64
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
type GAS struct {
interop.ContractMD
symbol string
decimals int64
}
type GAS struct {
interop.ContractMD
}

Keep it as simple as possible. Return constants from symbol and decimals.

Comment on lines +149 to +150
stackitem.NewByteArray(from.BytesBE()),
stackitem.NewBigInteger(amount),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
stackitem.NewByteArray(from.BytesBE()),
stackitem.NewBigInteger(amount),
stackitem.Make(from.BytesBE()),
stackitem.Make(amount),

Shorter, simpler.

Comment on lines +298 to +301
sig, ok := vectorRaw[j].Value().([]byte)
if !ok {
panic(fmt.Errorf("unexpected %d signature value in %d signatures vector: %s expected, %s given", j, i, stackitem.ByteArrayT, sigsVectorsRaw[j].Type()))
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Length check?

sigVectors = append(sigVectors, vector)
}

cnrListRaw := ic.DAO.GetStorageItem(m.ID, append([]byte{containerPlacementPrefix}, cID[:]...))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cID.BytesBE(), ditto everywhere.

}

func isSignedBySNs(ic *interop.Context, contract util.Uint160, cID util.Uint256, placementVectorsNumber int) error {
if l := len(ic.Tx.Scripts); l != 1 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if l := len(ic.Tx.Scripts); l != 1 {
if l := len(ic.Tx.Signers); l != 1 {

For proper RPC test invocations work.

}

acc := hash.Hash160(verifScript(contract, cID, placementVectorsNumber))
if !ic.Tx.Signers[0].Account.Equals(acc) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if !ic.Tx.Signers[0].Account.Equals(acc) {
if !ic.Tx.Sender().Equals(acc) {

Comment on lines +94 to +96
golang.org/x/mod v0.32.0 // indirect
golang.org/x/text v0.34.0 // indirect
golang.org/x/tools v0.41.0 // indirect
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants