feat: implement SNIP-29 (issue #745) #768
Conversation
|
Thank you @Salonii02! |
|
Hi @Salonii02! |
Okay Thanks!! No worries, will wait for the review. |
Hey @thiagodeev Did you get chance to review my changes |
|
Hey! Sorry @Salonii02, not yet. All PR reviews are still paused until the final rpc v0.9 release. |
|
Looking forward to this landing! |
|
Hey @Salonii02! |
Okay thanks!! Looking forward to it |
thiagodeev
force-pushed
the
implement-SNIP-29-issue-745
branch
3 times, most recently
from
a44fda4 to
e05372f
Compare
thiagodeev
force-pushed
the
implement-SNIP-29-issue-745
branch
from
d2f2a71 to
1164214
Compare
This file is planned to be removed in the future, so no need to add more tests here. We can use the mockgen pkg intead
…erface We are not using this interface anywere, nor the mock
…Paymaster and removing environment enum
…or clarity and consistency
…on for existing tests
- The test mocks the paymaster environment and checks the response against expected values. - Ensured proper error handling and JSON response validation within the test.
- Moved the TxnStatus enum and its JSON marshaling/unmarshaling methods from types_paymaster.go to tracking_id.go for better organization.
- Since the file only has error types, makes sense to rename it
- It has no other use inside the code than asserting implementation
- Made the Tip field in FeeMode optional, and returns nothing if it is ommited. No magic side effects in the MarshalJSON method. - Updated comments for better understanding of default behaviors. - Adjusted JSON test data to reflect changes in the FeeMode structure.
- Introduced a new example in the`examples/` folder demonstrating the usage of the paymaster provider.
- Added a new `deploy.go` file showing how to use the paymaster to deploy an account sending a `deploy` txn. - Introduced a constant for the AVNU paymaster URL to improve code readability.
- Added a new `deploy_and_invoke.go` file showing how to use the paymaster to deploy an account and invoke a txn by sending a `deploy_and_invoke` txn.
- Introduced a new function to retrieve the AVNU_API_KEY from the environment. - Updated paymaster example files to load the AVNU_API_KEY for deployment and invocation. - Modified the README to include instructions for setting the AVNU_API_KEY in the .env file.
- Added a new section in the main README to link to the paymaster example. - Updated examples README to provide instructions on how to interact with the paymaster and send transactions.
- Created a new GitHub Actions workflow for running integration tests on the paymaster module. - Updated the existing test workflow to include building the paymaster directory.
There was a problem hiding this comment.
Hey @Salonii02, my work is finished!
Paymaster is working and well tested.
Here's my review as promised:
I can't help but assume you used AI for most of your work.
Even the original PR description seems mostly AI-generated. Please correct me if I'm wrong.
Some errors of the initial code:
- The initial
NewPaymasterClientfunction initializes a paymaster using therpc.NewProviderfunction, which expects a Starkent RPC provider, not a paymaster. This alone is sufficient to make it impossible to instantiate a paymaster with this function, because inside therpc.NewProviderfunction, there's a call to thestarknet_specVersionRPC method, which would fail since the paymaster is not a Starknet RPC provider. - You created a lot of boilerplate code that Starknet.go already has. The entire
OutsideExecution...part could be handled by thetypedDatapkg, which is our implementation for the SNIP-12, used by the paymaster to sign the off-chain data - All these errors did not appear due to a simple fact: your tests were almost useless.
Take a look again at the TestPaymasterTypes test. You are simply manually instantiating some types and checking if they are not nil. Of course they are not nil! You manually initialized them.
There is not a single test that does a real call to a paymaster and asserts the client's functionality. If you have done a real interaction with a Paymaster using your code, you would see that it was not working.
My conclusion is: either you did not pay enough attention to the Paymaster feature and also need to improve your testing mindset, or worse, you used AI for almost everything and did not even check the result (I think this is the case).
So, my advice for you:
- You can't use AI to do your work; this will disrupt your learning.
- Use AI to help you understand things, or to do repetitive tasks that you already know how to do.
- Always check your code with real scenarios, or the closest possible ones. If you have not tested it, you can't know it works.
- When contributing to a project, take time to search the codebase for what is already built so that you can use it instead of wasting time creating more code. But it's okay to also ask for help.
Hope that helps you in your journey. Thanks again for the kickstart!
The code will now be reviewed by my other teammates. It would be great to have your review on my code as well!
github-project-automation
bot
moved this from 🔖 To do
to 👀 In review
in [Nubia] Starknet.go
|
Hey @Salonii02! I'll merge this once all the changes have been reviewed |
3 participants
Summary
This pull request introduces supporting methods for the SNIP-29 Applicative Paymaster API, enabling off-chain paymaster integration for sponsored and flexible transaction fee payments.
This PR closes #745.
Key Changes
paymasterpkg was created in the root of the repository to provide a dedicated interface for the paymaster API.paymaster_isAvailablepaymaster_getSupportedTokenspaymaster_buildTransactionpaymaster_executeTransactionpaymaster_trackingIdToLatestHashpaymasterexample in theexamplefolders showing how to use it.