feat(ui): apply diamond architecture to staking page

[bojan07]

Resolves #785

[ubiquibot]

• cd4ce71
• c086a17
• e7dc29f
• de99518
• 9e07817
• eb6e495
• 61ee7f7
• 6972f9c
• 57bc6e1
• 1ce1d35
• 9ad50ee
• 18ee556
• f5ac79b
• 5e1a071
• 8da767d

[rndquu]

@bojan07 is it ready for review?

[bojan07]

@bojan07 is it ready for review?

Not yet, I have found some bugs when running.. still on it.. will be ready for review soon

[bojan07]

@rndquu @molecula451 ready for a review

[bojan07]

@rndquu when running the UI, call revert exception error related to pair() function come out.
That's because I have added governanceMarket contract instance that is using sushiSwapPool contract instance as well as metaPool contract instance to protocol contracts, but when deploying contracts in anvil testnet, depoyed sushiswappool address is not set in managerFacet, so when calling sushiSwapPoolAddress function in frontend, it returns zero address. hope this helps your review process

[bojan07]

@rndquu when running the UI, call revert exception error related to pair() function come out. That's because I have added governanceMarket contract instance that is using sushiSwapPool contract instance as well as metaPool contract instance to protocol contracts, but when deploying contracts in anvil testnet, depoyed sushiswappool address is not set in managerFacet, so when calling sushiSwapPoolAddress function in frontend, it returns zero address. hope this helps your review process

[rndquu]

1. Naming should be as explicit as possible. This property should be something like sushiPoolGovernanceDollarLp (we have a convention of not using tokens names like UAD, UBQ, etc.. but rather full names Dollar, Governance, etc...). The same thing applies to metaPool which should be smth like curveMetaPoolDollarTriPoolLp (so that it was obvious from the 1st glance what is this contract about)
2. There should not be any errors on start. Either hardcode sushi swap pool and curve metapool addresses on the frontend either update the deployment script. The purpose of this issue is to make staking back to work. We'll refactor hardcoded values later.
3. Even when I hardcoded all of the addresses (sushi swap pool , curve meta poll) I got the following error for the address which doesn't have any staked tokens (pls fix):

[bojan07]

3. Even when I hardcoded all of the addresses (sushi swap pool , curve meta poll) I got the following error for the address which doesn't have any staked tokens (pls fix):

Oh.. I also hardcoded those address and checked, but I couldn't find that error case
erc1155BalanceOf function is only imported in use-balances file, and that error only can be happened when creditNft or stakingShare contract in protocol contracts is null.. but those can't be null.. weird
I would like you to check it out again

[bojan07]

2. There should not be any errors on start. Either hardcode sushi swap pool and curve metapool addresses on the frontend either update the deployment script. The purpose of this issue is to make staking back to work. We'll refactor hardcoded values later.

I have read this review again. you are asking me to update smart contract deployment script? If yes, I think it will be another issue related to smart contract, not to ui. What do you think?

[bojan07]

I'm going to open a new PR for credit page issue with the work for uniswap router issue. And hope you to update me if you find something useful for that uniswap router issue

[bojan07]

And if there is no problem with the current codebase, it's good to merge this PR for staking page issue, I think

[bojan07]

@molecula451 last two commits(88c1b46, 9e07817) in this PR involves the work for the diamond architecture implementation to credit page, except for removing all non-null assertion operators

[0x4007]

Overall the code seems fine. Unfortunately I'm not familiar with the context because I haven't worked with this code in several months.

Variable names must be changed according to the link I shared.

[bojan07]

Overall the code seems fine. Unfortunately I'm not familiar with the context because I haven't worked with this code in several months.

Variable names must be changed according to the link I shared.

Got it. Let me change the names according to the link

[rndquu]

@bojan07

Regarding your questions in telegram DM

I'm reaching out to you because PR804 is still open, not merged yet after you reviewed a few days, even though I fixed all as directed

No, not everything is fixed. There's a huge red error if you open the credits page.

Could you possibly help me to finish that issue by checking the PR again as a top priority?

I will definitely review the PR again when the credits page is opened without any errors.

Rerarding this PR:

1. As far as I understand this PR resolves 2 issues: UI: apply diamond to staking page #785 and UI: apply diamond to credits page #786. This is not really a good practice because a single PR should correspond to a single issue in order not to transforms PRs in a mess. We can put up with it right now, but for future PRs pls follow the "1 PR => 1 issue" rule.
2. Pls fix all of the pavlovcik's comments
3. Regarding this error. You said that you can't solve it so we asked @molecula451 to help with it. So we're waiting for some more info on that issue. We can't merge a PR with runtime errors.

[rndquu]

Approving this one because the original issue spec is implemented.

Credits page error will be solved as a part of this issue.

[molecula451]

it all points that the Uniswap router thing is a package issue that either we have to solve on the front end (on another issue) but it was not really introduced by these changes

[molecula451]

@rndquu it seems incredible but here is the bug https://github.com/Uniswap/sdk-core/blob/main/src/chains.ts#L1 the package is used by Uniswap-Smart Router, we have to clean many of the lines at useRouter() this package is not supporting anvil local testnet nor any "local" testnet at all as per see not even 8545 ganache, what we can do is something similar to #788 where we forked and then update, this package altho support mainnet, as when you switch mainnet big ### "Network Error" dissapear

[molecula451]

ubiquity-dollar/packages/dapp/components/lib/hooks/use-router.ts

Line 27 in 000bdb6

const router = new AlphaRouter({ chainId: 1, provider: provider });

this line make use of one of the chains at https://github.com/Uniswap/sdk-core/blob/main/src/chains.ts#L1 , but when we switch to local anvil (where our building is) it pops Network Error

so updating part of the package, and replacing chainId: localAnvil should do, in any case this whole useRouter() should be refactored, no part of this issue

[molecula451]

@bojan07 can you make use of

ubiquity-dollar/packages/dapp/.env.example

Line 4 in 000bdb6

NEXT_PUBLIC_DEBUG=

adding a conditional to the changes so are only visible at "DEBUG" env var?

[molecula451]

Approving this PR because the contributor successfully met the specs, all the bugs and pop ups are coming up from old code

[bojan07]

Thanks for your review. @molecula451 Really professional and detailed 👍
Thanks @rndquu @pavlovcik