-
Notifications
You must be signed in to change notification settings - Fork 24
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
docs recipe #60 #67
docs recipe #60 #67
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me ! Thanks for the detailed explanation, will chime in @0xquantum3labs for review
after this is updated i will leave it to @Nadai2010 to take a look and see if this works out of the box from a fresh clone 😄 |
on it ser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @suhas-sensei, thank you for your contributions!
I agree with @jrcarlos2000's review. The added code can be somewhat confusing, and for the specific contract, some parameters seem to be missing. For example, the set_greeting
method appears to lack the amount
parameter. Additionally, it would be beneficial to add documentation clarifying the changes, such as correctly configuring the chain from SupportedChain
using mainnetFork
in the scaffold.config
file. Otherwise, it can lead to network failures, even if the data retrieval is technically correct.
The screenshots below highlight the issues:
Once these changes are addressed, we will review again. Thanks for your efforts and input!
yeah, i'll look into the requested changes, thanks for the review. |
hello @suhas-sensei , any updates on this PR ? Thanks ! 🤝 |
yes, I'm sending an update today |
gm @suhas-sensei how is it going? just wanted to remind you that the OD hack is ending soon |
yes im really sorry for the delay, sending in a couple of hours. |
Tested out the wallet connections but being honest, couldn't try testing the contract being deployed as I didn't have enough experience with Scaffold-Stark. With what I learned in the past 3 days, I worked up a recipe that ensures wallet connections work on the forked chain, as shown in the screenshots. Thank you for giving me this great learning opportunity! |
have a look ser @metalboyrick |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi thanks for the new isFork contribution @suhas-sensei
In point 2, I see that you add update in the scaffold.config with const mainnetFork
, you must simply import from ‘supportedChain.ts’ this file is the one that controls that you can automate the network in [chains.maiinetFork], here is a sample
ah guess im bad at refactoring, just a sec |
Thank you @suhas-sensei can also indicate that in the .env.example you should create a NEXT_PUBLIC_PROVIDER_URL=http://127.0.0.1:5050 Otherwise it will give an error in the data when connecting to the rpc from the wrong network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good for me, thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Docs recipe for "Developing on a fork“
Fixes #60
Types of change
Added new recipe documentation that outlines how to: