Skip to content
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

Root / Remove Typechain #1677

Merged
merged 265 commits into from
Sep 24, 2024
Merged

Root / Remove Typechain #1677

merged 265 commits into from
Sep 24, 2024

Conversation

adamgall
Copy link
Member

@adamgall adamgall commented May 8, 2024

This is the root branch where all efforts to fully remove TypeChain from the codebase will be gathered.

Testing Instructions

The intention of this PR's "manual testing" is to touch every place in the app where data is read from, or written to, the blockchain, and ensuring that all goes as expected.

If you're reading this...

  • Please dig into some of the "Testing flows" below. As you run across code that might not be covered by any one of these existing flows, please add more testing flow paths into the tree to capture the spirit our stated intention!
  • As you run across bugs that you believe were caused by this PR, please fix and commit as you see fit.
  • As you run across bugs that you believe existed on develop already, please open an issue in our project board.

Testing flows

  • Create Safe ✅
    • Multisig (tx, dash)
    • ERC20 ✅
      • New Token (tx, dash)
      • Wrap existing Token ✅
        • Is already IVotes (tx, dash)
        • Is not already IVotes (tx, dash)
    • ERC721 ✅
  • Delegate voting tokens ✅
    • Azorius ✅
      • Tokens (tx, dash)
      • NFTs (not a thing)
  • Create proposal ✅
  • Vote on proposal ✅
  • Create child Safe ✅
    • From Azorius ✅
      • From Tokens (dash)
        • To Multisig (tx)
        • To Azorius Tokens (tx)
        • To Azorius NFTs (tx)
      • From NFTs ✅ (dash)
        • To Multisig (tx)
        • To Azorius Tokens (tx)
        • To Azorius NFTs (tx)
    • From Multisig ✅ (dash)
      • To Multisig (tx)
      • To Azorius Tokens (tx)
      • To Azorius NFTs (tx)
  • Initiate a Freeze ✅ (unchecked boxes below were previously blocked because menu option hadn't been showing, but successful clawback tests thereafter on safes starting from scratch show that freezes in those contexts do pass, when the menu option does show)
    • on Multisig Child from Azorius Tokens parent (tx,dash)
    • on Multisig Child from Azorius NFTs parent ()
    • on Multisig Child from Multisig parent (tx,dash)
    • on Azorius Tokens Child from Azorius Tokens parent (tx,dash)
    • on Azorius Tokens Child from Azorius NFTs parent ()
    • on Azorius Tokens Child from Multisig parent (tx,dash)
    • on Azorius NFTs Child from Azorius Tokens parent ()
    • on Azorius NFTs Child from Azorius NFTs parent ()
    • on Azorius NFTs Child from Multisig parent (tx,dash)
  • Create Clawback proposal on Frozen DAO ✅
    • on Multisig Child from Azorius Tokens parent (tx,dash)
    • on Multisig Child from Azorius NFTs parent (tx,dash)
    • on Multisig Child from Multisig parent (tx,dash)
    • on Azorius Tokens Child from Azorius Tokens parent (tx,dash)
    • on Azorius Tokens Child from Azorius NFTs parent (tx,dash)
    • on Azorius Tokens Child from Multisig parent (tx,dash)
    • on Azorius NFTs Child from Azorius Tokens parent (tx,dash)
    • on Azorius NFTs Child from Azorius NFTs parent (tx,dash)
    • on Azorius NFTs Child from Multisig parent (tx,dash)
  • Create proposal template ✅
  • Create proposal from template ✅
  • Create proposal from all ways to do so in DAO Settings page ✅
    • Multisig ✅
    • Update safe name ✅
      • Azorius Tokens
      • Azorius NFTs
    • Update snapshot space ✅
      • Azorius Tokens
      • Azorius NFTs
      • Multisig

Observed Issues

Screenshot 2024-09-17 at 13 48 19
  • Funky stuff's happening on 2nd time initiate freeze on separate subsafe
  • Subsafe's freeze status does not reliably show. Sometimes countdown is missing
  • Multisig proposal transaction hash before execute is misleading
    - freeze votes threshold still allows input more than parent safe signer threshold fixed
  • Another intermittent error when loading up the proposals page (but inconsistent as a refresh "solves" it):
Screenshot 2024-09-04 at 15 35 51
  • "Go Home" button doesn't work from Error boundary page =/
  • Nft safes with multiple nfts might have weird behaviour on (create child safe) proposal details page
  • Still think we need a louder, probably more persistent, call to Delegate voting tokens, as a lot of functionality is blocked until delegation happens, and it's not always clear why

…om-useSafeContracts

Remove safeFactoryContract from useSafeContracts
@adamgall adamgall self-assigned this May 8, 2024
Copy link

netlify bot commented May 8, 2024

Deploy Preview for fractal-dev ready!

Name Link
🔨 Latest commit 9b04514
🔍 Latest deploy log https://app.netlify.com/sites/fractal-dev/deploys/66477b0a1238f600083e83b5
😎 Deploy Preview https://deploy-preview-1677.app.dev.decentdao.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

adamgall and others added 16 commits May 8, 2024 15:40
…#1657)

* Add ModuleProxyFactory ABI to project

* Remove ModuleProxyFactory from FractalContracts and BaseContracts and then work through it all
* Remove GnosisSafeL2 typechain from TxActions

* Remove GnosisSafeL2 typechain from DaoTxBuilder

* Remove GnosisSafeL2 Typechain from TxBuilderFactory and Builder classes

* Finish removing GnosisSafeL2 Typechain from project

* Remove comment
* Remove all instances of MultiSend objects from FractalContracts, BaseContracs, useSafeContracts, etc

* Remove the usul typechain-types assets

* Fix import order
@adamgall adamgall closed this May 15, 2024
@adamgall adamgall reopened this May 15, 2024
@adamgall adamgall assigned DarksightKellar and unassigned adamgall and mudrila Sep 18, 2024
Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming there's no upcoming prod releases - I think we should be good to merge it now. If there will be something urgently needed for main - we can go through hot-fix branch targetting main branch
@adamgall @DarksightKellar @Da-Colon WDYT?

@adamgall
Copy link
Member Author

Assuming there's no upcoming prod releases - I think we should be good to merge it now. If there will be something urgently needed for main - we can go through hot-fix branch targetting main branch @adamgall @DarksightKellar @Da-Colon WDYT?

You know what, yes let's do it

@Da-Colon
Copy link
Contributor

Let's do it

@Da-Colon Da-Colon self-requested a review September 24, 2024 19:51
@adamgall adamgall merged commit f080210 into develop Sep 24, 2024
7 checks passed
@adamgall adamgall deleted the root/remove-typechain branch September 24, 2024 19:52
@adamgall
Copy link
Member Author

adamgall commented Sep 24, 2024

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request maintenance Keep the lights on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants