Deployed mock contracts #100 and approve scripts to aid swaps #99 #108
Conversation
blackbeard002
changed the title
…ress to contract's address
There was a problem hiding this comment.
Hey @blackbeard002 I was reviewing your PR, and I couldn't find the approval scripts referred on the #99, only the deploy script was committed, could you check that, please?
And the indentation of the code you committed is very different than the standard we are using. We are currently using prettier in its standard config, which is set automatically when you add it:
{
"trailingComma": "es5",
"tabWidth": 4,
"semi": false,
"singleQuote": true
}
scripts/mockERC.ts
Outdated
There was a problem hiding this comment.
The file needs to be formatted following the protocol standard.
There was a problem hiding this comment.
Definition of done:
- Quotes fixed
- Deploy Script
- Mint Script
Detailed
- Change single quotes to double quotes.
- Split the file into a deploy script and a mint script for the Mock. This means you run the file and the contract is deployed to the selected network and nothing else. (params: contract as string and valid evm address, abi as any and signer as SignerWithAddress, return: the contract instance (See deploy script in utils))
- In the second script, make it an external function passing the information needed as parameters without an initializer. (params: contract as Contract, amount as bigint, receiver as string (verify using ethers if it's a valid address, return: response from call).
- Fix some comments that are not in the same pattern: lacking space between double slashes and lowercase letters.
- Add overall try catch to display the error correctly instead of breaking the code.
This will facilitate to implement any sort of codes in the future since it will be very modularized.
|
We are thinking from the implementor's perspective where he needs to approve his user assets so they can swap their assets without transferring when the trade is created (btw, the trade created will not check for approvals, it will assume the approvals exist). |
|
yeah got the context now |
|
Hey @0xneves ready for review. |
scripts/mockERC.ts
Outdated
There was a problem hiding this comment.
Definition of done:
- Quotes fixed
- Deploy Script
- Mint Script
Detailed
- Change single quotes to double quotes.
- Split the file into a deploy script and a mint script for the Mock. This means you run the file and the contract is deployed to the selected network and nothing else. (params: contract as string and valid evm address, abi as any and signer as SignerWithAddress, return: the contract instance (See deploy script in utils))
- In the second script, make it an external function passing the information needed as parameters without an initializer. (params: contract as Contract, amount as bigint, receiver as string (verify using ethers if it's a valid address, return: response from call).
- Fix some comments that are not in the same pattern: lacking space between double slashes and lowercase letters.
- Add overall try catch to display the error correctly instead of breaking the code.
This will facilitate to implement any sort of codes in the future since it will be very modularized.
There was a problem hiding this comment.
Definition of done:
- Quotes fixed
- Approve Script
Detailed:
- Change single quotes to double quotes.
- Remove the main() initializer. Rename the export function into
approve. Then add parameters: contract instance as Contract (Signer will already be msg.sender in this case), spender as valid EVM address, amountOrId as bigint, returns transaction response). - Remove any sorts of logs.
- Fix some comments that are not in the same pattern: lacking space between double slashes and lowercase letters.
- Add overall try catch to display the error correctly instead of breaking the code.
This way the script can be used for both ERC20 and ERC721 functions dynamically.
There was a problem hiding this comment.
Hey @0xneves,
Also about the single quote to double quotes changes, the prettier config standard you mentioned earlier has "singleQuote": true. So it was all switched to single quotes. should this value be removed or something from the prettier yml? For now I'm changing it to doubleQuote:true since you asked me to change single quotes to double quotes.
There was a problem hiding this comment.
Hey @0xneves, Also about the single quote to double quotes changes, the prettier config standard you mentioned earlier has "singleQuote": true. So it was all switched to single quotes. should this value be removed or something from the prettier yml? For now I'm changing it to doubleQuote:true since you asked me to change single quotes to double quotes.
Hey @blackbeard002
The mentioned issue was fixed in #128
|
Hey @blackbeard002 Talked to you in discord, let's speed this up as it is review for almost a month now 🥵 |
|
yeah I'm on it |
|
Hey @0xneves, ready for review. I've made changes according to the info and params mentioned here on this PR chat. Lemme know if any changes are needed. |
Hey @0xneves, mock.ts script to deploy and mint ERC20 and ERC721.
This PR solves #100 and #99 . Ran the scripts on local hardhat.