WIP - New contracts and contracts-sdk packages#1264
WIP - New contracts and contracts-sdk packages#1264
Conversation
…pdated contracts-sdk
SnickerChar
left a comment
SnickerChar
left a comment
There was a problem hiding this comment.
This was a long review but I don't remember finding anything that requires a change right now. Looks good and I like the write ups and readmes.
|
|
||
| // #0xfE3eEB4ba0FA4BD3b398711AF18908136f5bf430 | ||
| export const privateKey1 = EVMPrivateKey( | ||
| "87f0ccf57f9778b5b2e7b62ceddd9530dd6daa6efef6cd03c986d4cc48e2d62b", |
There was a problem hiding this comment.
These are just keys in Hardhat, right?
| protected contractAbi: ethers.InterfaceAbi; | ||
| protected hasSigner = false; | ||
|
|
||
| constructor( |
There was a problem hiding this comment.
One thing that we decided we did wrong a long time ago was not make a distinction between readable contracts and writable contracts. I think BaseContract here could still be the base, but we remove providerOrSigner and just make it "provider" (no hasSigner). Then we can make BaseWritableContract extends BaseContract and takes a Signer (and a Provider?). Then the hard part though; you can't extend both ReadableFooContract and BaseWritableContract.... Hmm. Never mind I guess for now. Head's kind of foggy
| @@ -0,0 +1,427 @@ | |||
| import { | |||
There was a problem hiding this comment.
Isn't this just a general ERC1155Contract? I always kind of got confused on the need for a separate "reward" contract version.
| @@ -0,0 +1,19 @@ | |||
| import { ERC7529ContractError } from "@snickerdoodlelabs/objects"; | |||
There was a problem hiding this comment.
I just realized that you have stuff in src-new; since you moved all the old stuff to a new package contracts-old, shouldn't this just be src?
| @@ -0,0 +1,121 @@ | |||
| import { | |||
There was a problem hiding this comment.
I think this is part of the old protocol, and won't be needed in the new contracts package
| @@ -0,0 +1,16 @@ | |||
| import { BigNumberString, EVMAccountAddress } from "@snickerdoodlelabs/objects"; | |||
There was a problem hiding this comment.
I think this can go away as well, stake for rank is currently dead.
| public abi?: string, | ||
| ) {} | ||
|
|
||
| wait(): ResultAsync<ethers.TransactionReceipt, TransactionResponseError> { |
| @@ -0,0 +1,1170 @@ | |||
| export default { | |||
There was a problem hiding this comment.
You should remove these ABIs, consent contracts are not part of the system anymore.
| Contains ad wallet contracts | ||
|
|
||
| ### [token](/packages/contracts/contracts/token/README.md) | ||
| ### [user-wallet](/packages/contracts/contracts/user-wallet/README.md) |
There was a problem hiding this comment.
I'm a little confused, we have the smart-wallet contract above; is that the name we're using, or are we sticking with user wallet?
| /// @param to The receiver address | ||
| /// @param amount The amount to transfer in wei | ||
| function transferERC20(address tokenAddress, address to, uint256 amount) public onlyOwner { | ||
| require(IERC20(tokenAddress).transfer(to, amount), "ERC20 transfer failed"); |
There was a problem hiding this comment.
These methods I thought were going to be "withdraw" methods
…eturn OperatorAndPoint object
…net readme, tasks
…ickerdoodleLabs/protocol into feature/new-contracts-package
| # install contracts package dependencies | ||
| RUN npm install | ||
| RUN npm install -D @nomicfoundation/hardhat-toolbox solidity-coverage dotenv | ||
| RUN npm install --force |
There was a problem hiding this comment.
needs force otherwise it errors with ethers version mismatch
…ickerdoodleLabs/protocol into feature/new-contracts-package
4 participants
Release Notes
JIRA Link
Summary:
Includes new contracts and contract-sdk packages for the revamped contracts.
Intended results:
Potential Failures:
Relevant Metrics/Indicators:
Testing Notes:
Pre-Flight Checks