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

[Framework Add] implement payable #990

Merged
merged 16 commits into from
Jun 5, 2024

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Mar 11, 2024

This pr focus on implement NEP11 and NEP17 callback standards.

Wait for neo-project/proposals#169

@Jim8y Jim8y added the blocked label Mar 11, 2024
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Looks good, currently it is more informal

src/Neo.Compiler.CSharp/ContractManifestExtension.cs Outdated Show resolved Hide resolved
src/Neo.Compiler.CSharp/ContractManifestExtension.cs Outdated Show resolved Hide resolved
Comment on lines 172 to 174
onNEP11PaymentMethod.Parameters[0].Type == ContractParameterType.Hash160 &&
onNEP11PaymentMethod.Parameters[1].Type == ContractParameterType.Integer &&
onNEP11PaymentMethod.Parameters[2].Type == ContractParameterType.Any;
Copy link
Member

Choose a reason for hiding this comment

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

Parameter names should also be checked, they are a part of the standard. Take a look at https://github.com/nspcc-dev/neo-go/blob/319880e2014ebbe8abd8ce58dcf212869c22604b/pkg/smartcontract/manifest/standard/comply.go#L68.

Copy link
Member

Choose a reason for hiding this comment

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

This can be very strict, maybe we want to call to our token myTokenId, I think that names are not needed

Copy link
Member

Choose a reason for hiding this comment

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

@roman-khimov, what do you think?

Choose a reason for hiding this comment

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

I'm thinking of transfer(to Hash160, from Hash160, amount Integer, id ByteString, _ Any) and Comply() preventing it.

src/Neo.Compiler.CSharp/ContractManifestExtension.cs Outdated Show resolved Hide resolved
src/Neo.SmartContract.Framework/NEPStandard.cs Outdated Show resolved Hide resolved
shargon
shargon previously approved these changes May 22, 2024
@Jim8y
Copy link
Contributor Author

Jim8y commented May 22, 2024

Weird, test runs OK on my macos

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Is the PR still blocked? What are the plans?

@shargon
Copy link
Member

shargon commented May 23, 2024

Is the PR still blocked? What are the plans?

Let's wait for merge the proposal

shargon
shargon previously approved these changes May 23, 2024
Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Wait for merge the proposal

@Jim8y Jim8y requested a review from a team May 24, 2024 11:21
@Jim8y Jim8y merged commit bbda133 into neo-project:master Jun 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants