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

Allow to obtain the current tx #1432

Closed
wants to merge 5 commits into from

Conversation

shargon
Copy link
Member

@shargon shargon commented Jan 30, 2020

Close #1431

@shargon shargon requested review from erikzhang and belane January 30, 2020 17:35
belane
belane previously approved these changes Jan 30, 2020
@shargon shargon requested review from Tommo-L and belane February 2, 2020 09:34
belane
belane previously approved these changes Feb 2, 2020
Tommo-L
Tommo-L previously approved these changes Feb 2, 2020
@shargon
Copy link
Member Author

shargon commented Feb 2, 2020

dotnet format is failing without changes...

@superboyiii
Copy link
Member

@shargon Will you apply it on devpack-dotnet?

@shargon
Copy link
Member Author

shargon commented Feb 3, 2020

Will you apply it on devpack-dotnet?

Yes, take a look neo-project/neo-devpack-dotnet@060d300

@shargon shargon added the Blocker Issues that are blocking other issues. Check issues details to see what it is blocking. label Feb 3, 2020
@shargon shargon dismissed stale reviews from Tommo-L and belane via 610619e February 3, 2020 08:35
@shargon
Copy link
Member Author

shargon commented Feb 4, 2020

@erikzhang could you merge this, the error is not related to this PR

@erikzhang
Copy link
Member

Why not use System.Runtime.GetScriptContainer?

@shargon
Copy link
Member Author

shargon commented Feb 5, 2020

As a developer perspective, we return an Array of stack items, but the developer can't know if it's a block or Transaction, with this method, we always know that it's a Transaction, maybe we should remove the GetScriptContainer

Wwhat is the behaviour of this if it's not a Block?

Block block=(Block)Runtime.GetScriptContainer();

@erikzhang
Copy link
Member

If it's a block, it won't call GetScriptContainer.

@shargon
Copy link
Member Author

shargon commented Feb 5, 2020

If it's a block, it won't call GetScriptContainer.

Always is a TX except in ConsensusPayload and Block right? So do you think that it's not needed because with SmartContract always will be a Transaction?

Is not better to create two syscalls and add GetCurrentBlock?

@Tommo-L
Copy link
Contributor

Tommo-L commented Feb 9, 2020

Is not better to create two syscalls and add GetCurrentBlock?

For me, I'd like to see that we can add it, as it'll be more friendly to contract developers.

@superboyiii superboyiii mentioned this pull request Feb 11, 2020
@erikzhang erikzhang closed this Feb 21, 2020
@shargon shargon deleted the get-current-tx branch February 21, 2020 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker Issues that are blocking other issues. Check issues details to see what it is blocking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to obtain the current tx
6 participants