MiniWallet Smart Contracts Testing and Deployment

[johnwhitton]

This Issue tracks the next steps for the SMS-Wallet Smart Contracts (Chaincode)
It follows on from Pull Request #1 enhancing and hardening testing, deployment and adding in additional ERC777 functionality

ToDo

Priority Tasks

• Rename AssetManager.sol to MiniWallet.sol
• Add deposit functionality to approve
• Change all requires to if tests and error messages
• Testing
  • Additional tests for require, modifiers and pausable functions
  • Additional tests when adding operators over thresholds
  • Additional tests for depositing or approving funds over global limits
  • Refactor testing
• Deployment
  • Review deployment best practices for proxies including using hardhat-deploy instead of openzepplin upgrades
  • Deploy to Testnet
  • Verify contract on Testnet
  • Test Contract Upgrade Process
• Go Live Checklist
  • Testnet Deploy
  • Testnet MinServer Integration
  • Production Deploy
• Add multicall functionality for batch requests

Additional Tasks

• Add in support for ERC777. References ERC-777 Sample Code, EIP-777, OpenZepplin ERC777 Documentation, OpenZepplin ERC777.sol, OpenZepplin ERC777 Example Implementation, ERC777 Masters Thesis
  • Initial code is was done in commit b467f85 on branch jw-erc-777
• Documentation: add how to run coverage server and
  • Hardhat support for Harmony
• Enhancements
  • Replace all requires with if tests reverts and custom errors
  • Could introduce warning events when changing thresholds if user has a balance over the new threshold
• Optimizations
  • Review contract optimization best practices
• Security
  • add yarn.lock to gitIgnore comments here

Outstanding Issues/Clarifications

• ERC721 safeTransferFrom not working in AssetManager.ts (using transferFrom)
• ERC1155 safeTransferFrom needs "0x" passed for data in AssetManager.ts ("" in AssetManager.sol)
• Review errorHanding for transfer (currently capture failures for ERC20 but not ERC721 and ERC1155).
• Deploy failing with unpredicatable gas limit was caused when there was a bug in the contract (not setting the admin role before setting operators) if this happens again may need to apply this workaround to fix gas price