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

Update opcodes #216

Merged
merged 13 commits into from
Mar 30, 2020
Merged

Update opcodes #216

merged 13 commits into from
Mar 30, 2020

Conversation

shargon
Copy link
Member

@shargon shargon commented Mar 23, 2020

Close #209

  • Update Assert and Abort tests
  • Create UT for HexToBytes

@shargon shargon requested a review from lightszero March 23, 2020 21:14
lightszero
lightszero previously approved these changes Mar 24, 2020
src/Neo.SmartContract.Framework/Helper.cs Outdated Show resolved Hide resolved
src/Neo.SmartContract.Framework/SmartContract.cs Outdated Show resolved Hide resolved
@shargon shargon requested a review from lightszero March 24, 2020 11:10
@shargon
Copy link
Member Author

shargon commented Mar 24, 2020

Now got this error @lightszero @erikzhang

Initialization method Neo.SmartContract.Framework.UnitTests.Services.Neo.IteratorTest.Init threw exception. System.TypeLoadException: Could not load type 'Neo.SmartContract.Framework.SmartContract' from assembly 'Neo.SmartContract.Framework, Version=3.0.0.0, Culture=neutral, PublicKeyToken=null' because the method 'Assert' has no implementation (no RVA)..

@shargon
Copy link
Member Author

shargon commented Mar 24, 2020

I can fix it removing the abstract keyword, but i am not sure if it will add a empty CALL with empty body after the call

d598207

@shargon shargon requested a review from erikzhang March 24, 2020 11:31
@lightszero
Copy link
Member

i will check it later.

@lightszero
Copy link
Member

lightszero commented Mar 24, 2020

rosyln build in unittest can't extend a class with extern method.

   class abc:SmartContract // if SmartContract have extertn method  will got a build error.

@shargon
so we move Abort and Assert to another class will fix this.


namespace Neo.SmartContract.Framework
{
public static class Debug
Copy link
Member Author

Choose a reason for hiding this comment

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

Runtime?

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 rename it.

@shargon
Copy link
Member Author

shargon commented Mar 24, 2020

@lightszero with empty body it's not possible to do that? discard the body if this method has OpCodeAttribute?

@lightszero
Copy link
Member

lightszero commented Mar 24, 2020

@lightszero with empty body it's not possible to do that? discard the body if this method has OpCodeAttribute?

is possible,but we use extern in framework everywhere.so i think we should contine use extern for abort and assert

erikzhang
erikzhang previously approved these changes Mar 25, 2020
@shargon
Copy link
Member Author

shargon commented Mar 25, 2020

nice!!!

@shargon shargon mentioned this pull request Mar 26, 2020
@erikzhang
Copy link
Member

Test failed.

@shargon
Copy link
Member Author

shargon commented Mar 26, 2020

I will revert the last change because it will require GasLeft

@ProDog
Copy link
Contributor

ProDog commented Mar 27, 2020

I have tested it passed.
Contract:

image

Invoke result:
for assert(true):

image
image

for assert(false):

image

for abort:

image

@ProDog
Copy link
Contributor

ProDog commented Mar 30, 2020

plz review it and merge code.

@ProDog
Copy link
Contributor

ProDog commented Mar 30, 2020

Can merge into master? @erikzhang @lightszero

@lightszero
Copy link
Member

Can merge into master? @erikzhang @lightszero

good for me

@lightszero lightszero merged commit 587b8c1 into master Mar 30, 2020
@lightszero lightszero deleted the update-opcodes branch March 30, 2020 07:52
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.

Sync new opcode: ASSERT, ABORT
4 participants