search

polymorpher / sms-wallet

SMS Wallet - non-custodial wallet bound to phone number
https://smswallet.xyz
6 stars 4 forks source link

Mini v0 #14

Open johnwhitton opened 2 years ago

johnwhitton commented 2 years ago

Version 0 work based on https://github.com/polymorpher/sms-wallet/issues/13

polymorpher commented 2 years ago

Can you gitignore everything under /deployment? No auto-generated code or local-environment specific stuff should be checked in

polymorpher commented 2 years ago

Can all Mini1155 and 721 related deployment and testing stuff be removed? They are launch templates other developers may use, and have nothing to do with SMS Wallet features. The contract themselves may be moved to separate places as well

polymorpher commented 2 years ago

Can you move all work-in-progress and MiniID related items to separate PR? Let's make one thing perfectly good, solid for production, and as minimal as possible in code, before moving on to the next.

polymorpher commented 2 years ago

There is still the issue unresolved, related to recording proxy, logic, and storage addresses for those hardhat deployed contracts, and fine-control on these contract's upgrade process. We can't just use deploy or upgrade without knowing or controlling each and every step what it is doing. Are they using factories and create2? Using EOA and create op code? In some situations we also need to control the address which the contracts are deployed to. We need to know all these. In other situations when we upgrade the contracts, the storage slots may mess up because of storage layout issues, and we need to test ahead of time. We cannot do this without complete visibility and fine-control over the deploy and upgrade process

polymorpher commented 2 years ago

Let's also move development notes to miniwallet/devlog/ with a README stating they are development notes for internal discussions, not meant for documentation or guiding users or developers.

We can polish and move things out to component-root folder after they are ready for production and presentable for external audience

polymorpher commented 2 years ago

I have two more files to read: NFTID.md and PROXY.md. I went through other files.

Let's break down this PR into multiple parts (each has its own PR):

  1. core mini-wallet (server, contract, tests) - these are ready for merge
  2. mini-wallet deployment and upgrade scripts - these needs some improvement on fine-control over deploy and upgrade process (most likely can't just use simple hardhat calls / plugins anymore) and storing (not just logging) the addresses of proxy, logic, and storage
  3. mini1155, 721, and related deployment scripts and tests - these need some sanitization and maybe some simplification on tests. They are pretty much ready for merge
  4. miniID related and other WIP stuff - these are incomplete
  5. documentations (NFTID.md, PROXY.md) - these need further review and some revision - I can also edit on the branch itself

The PRs can be made in chains (each PR's branch merge into the branch of previous PR) rather than all pointing to the base (main) branch, since each may be dependent on some of the work in previous PR

polymorpher commented 2 years ago

On a second thought - I think (2) can be merged in after using a better proxy, so we can get it up and running ASAP. We can improve this process in a later PR