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

Simplify interop services #1082

Closed
erikzhang opened this issue Sep 2, 2019 · 3 comments · Fixed by #1081
Closed

Simplify interop services #1082

erikzhang opened this issue Sep 2, 2019 · 3 comments · Fixed by #1081
Labels
Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. Ledger Module - The ledger is our 'database', this is used to tag changes about how we store information Ready-to-Implement Issue state: Ready to be implemented or implementation in progress
Milestone

Comments

@erikzhang
Copy link
Member

erikzhang commented Sep 2, 2019

Summary
Currently, our interop service API is very complicated. For example, to get a transaction, we need to call System.Blockchain.GetTransaction and then call Neo.Transaction.GetScript to get the fields of the transaction. Maybe we can simplify them.

Solution
In fact, we have an API that is very simple, that is System.Runtime.GetNotifications.

private static bool Runtime_GetNotifications(ApplicationEngine engine)
{
byte[] data = engine.CurrentContext.EvaluationStack.Pop().GetByteArray();
if ((data.Length != 0) && (data.Length != UInt160.Length)) return false;
IEnumerable<NotifyEventArgs> notifications = engine.Notifications;
if (data.Length == UInt160.Length) // must filter by scriptHash
{
var hash = new UInt160(data);
notifications = notifications.Where(p => p.ScriptHash == hash);
}
if (!engine.CheckArraySize(notifications.Count())) return false;
engine.CurrentContext.EvaluationStack.Push(notifications.Select(u => new VM.Types.Array(new StackItem[] { u.ScriptHash.ToArray(), u.State })).ToArray());
return true;
}

We can change all the APIs that get the data into this way. This way we can remove a lot of redundant APIs.

Where in software does this update applies to?

  • SDK
  • Other: NeoContract
@erikzhang erikzhang added the Discussion Initial issue state - proposed but not yet accepted label Sep 2, 2019
@erikzhang erikzhang added this to the NEO 3.0 milestone Sep 2, 2019
@shargon
Copy link
Member

shargon commented Sep 2, 2019

Totally agree, we should follow this way

@shargon
Copy link
Member

shargon commented Sep 3, 2019

I will change this #1081 in order to fit your pattern, it could save a lot of opcodes

@igormcoelho
Copy link
Contributor

Fully agree Erik, as long as Array behaves as Object, this will be fine.

@lock9 lock9 added Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. Ledger Module - The ledger is our 'database', this is used to tag changes about how we store information Ready-to-Implement Issue state: Ready to be implemented or implementation in progress and removed Discussion Initial issue state - proposed but not yet accepted labels Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. Ledger Module - The ledger is our 'database', this is used to tag changes about how we store information Ready-to-Implement Issue state: Ready to be implemented or implementation in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants