MMI-5405-investigate-and-solve-for-missing-product-on-stargate-to-ensure-we-have-full-coverage #304

bergarces · 2024-08-28T14:21:14Z

Added missing chains and metadata for Stargate V1 LP pools.
Added support for LP token staking for V1 and V2

bergarces · 2024-09-02T16:05:18Z

packages/adapters-library/src/adapters/protocols.ts

@@ -33,6 +33,7 @@ export const Protocol = {
  SparkV1: 'spark-v1',
  StakeWise: 'stakewise',
  Stargate: 'stargate',
+  StargateV2: 'stargate-v2',


Do we want stargate-v2 or do we want to group the new products inside stargate?

If we want to align our protocol ids with other providers, I don't think they have a stargate-v2.

I dont know what debank have....lets wait for the ticket

From their UI, it looks like everything is Stargate. Whether the API is the same or not, we don't know yet.

I've taken the safe approach and all of them are under Stargate now.

bergarces · 2024-09-02T16:06:19Z

packages/adapters-library/src/adapters/stargate-v2/common/staticChainData.ts

+        poolUsdcAddress?: string
+        poolUsdtAddress?: string
+        poolMetisAddress?: string
+        poolMethAddress?: string


There's no event or any contract I could find where these contract addresses live. They've been fetched from their docs.

https://stargateprotocol.gitbook.io/stargate/v/v2-developer-docs/technical-reference/mainnet-contracts

…roduct-on-stargate-to-ensure-we-have-full-coverage

bergarces · 2024-09-05T10:46:05Z

.../adapters-library/src/adapters/morpho-blue/contracts/factories/Metamorphofactory__factory.ts

This started crashing my local build due to the json file not matching with the class name.

oh ok, not sure what happened there they must have change the name...good spot

bergarces · 2024-09-05T10:47:30Z

packages/adapters-library/src/core/utils/caseConversion.ts

It was driving my nuts that every time we did something like v2 it will be converted to v-2, so I amended the implementation.

yes nice improvement :)

sonarcloud · 2024-09-05T10:51:29Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 30%)
26.8% Duplication on New Code (required ≤ 7%)

See analysis details on SonarCloud

jpsains · 2024-09-06T08:19:31Z

packages/adapters-library/src/adapters/stargate/products/farm-v2/stargateFarmV2Adapter.ts

+      if (
+        protocolTokenAddresses &&
+        !protocolTokenAddresses.includes(protocolToken.address)
+      ) {
+        return undefined
+      }


Do you see a future where we can drop having to do this filter inside the adapter? Wondering if the filtering could be done outside....

To be honest, I can't.

If we do it outside that means the return function will be something like Promise<ReturnType | undefined>, which doesn't make a lot of sense.

Moreover, we use that pattern pretty often internally as well, which means that if we don't get rid of it quickly and we chain the result to another array method, the first thing we'll have to do is check if it's undefined.

It's unfortunate that JS doesn't come with an array method that does that automatically.

jpsains · 2024-09-06T08:27:51Z

packages/adapters-library/src/adapters/stargate/products/farm-v2/stargateFarmV2Adapter.ts

+    return events.map((event) => {
+      const { amount } = event.args!
+
+      return {
+        protocolToken: {
+          address: protocolToken.address,
+          name: protocolToken.name,
+          symbol: protocolToken.symbol,
+          decimals: protocolToken.decimals,
+        },
+        blockNumber: event.blockNumber,
+        transactionHash: event.transactionHash,
+        tokens: [
+          {
+            address: protocolToken.address,
+            name: protocolToken.name,
+            symbol: protocolToken.symbol,
+            decimals: protocolToken.decimals,
+            type: TokenType.Underlying,
+            blockNumber: event.blockNumber,
+            balanceRaw: amount,
+          },
+        ],
+      }
+    })
+  }


This is not a blocker but i think you might have been able to use the transfer movements of the protocol token

const movements = await this.helpers.getErc20Movements({ protocolToken: underlyingLpToken, filter: { fromBlock, toBlock, from: userAddress, to: protocolTokenAddress, }, })

We have a helper

but please dont bother to update its not worth it

100% worth it if we have a helper. I'll update.

jpsains · 2024-09-06T08:31:36Z

packages/adapters-library/src/adapters/stargate/products/farm/stargateFarmAdapter.ts

+      const lpStakingContract =
+        protocolToken.lpStakingType === 'LpStaking'
+          ? LpStaking__factory.connect(
+              protocolToken.lpStakingAddress,
+              this.provider,
+            )
+          : LpStakingTime__factory.connect(
+              protocolToken.lpStakingAddress,
+              this.provider,
+            )


This is fine for now but what stopped you doing a separate adapter?

It was my initial thought, since it looked that some chains used one contract and some the other. So my plan was to make a common adapter and then extend for the very tiny different bits.

Then I saw that some chains (mainnet) used both depending on the pool.

Overall, it's a question of whether these ifs are annoying enough to justify a whole new adapter that will either have almost identical code or an abstraction with then several bits of code in protected functions reimplemented for each type.

No strong opinion either way, it's a grey area.

jpsains · 2024-09-06T08:34:27Z

packages/adapters-library/src/adapters/stargate/products/farm/stargateFarmAdapter.ts

+  /**
+   * There are no specific events for rewards, which occur as simple transfers whenever a Deposit or Withdrawal is actioned
+   * For this reason, we need to fetch all Deposit and Withdrawal events and then filter out the reward transfers
+   */


These are the type of scenarios, among others, are the reason why I dont want us to rule out having an indexer....hence why I want @xavier-brochard to at least explorer the indexer solution for ETH2

jpsains · 2024-09-06T08:36:14Z

packages/adapters-library/src/adapters/stargate/products/farm/stargateFarmAdapter.ts

+    const depositFilter = lpStakingContract.filters.Deposit(
+      userAddress,
+      protocolToken.poolIndex,
+      undefined,
+    )


Just to confirm are you saying rewards can be "claimed" even with deposits?

Yes, rewards get automatically claimed whenever user deposits or withdraws. They don't seem to have a method to claim them directly.

jpsains · 2024-09-06T08:37:47Z

packages/adapters-library/src/core/utils/caseConversion.ts

+// Copies lodash implementation of kebabCase and modifies it to treat version numbers as a single word
+function kebabCase(input: string) {
+  return words(lodashToString(input).replace(/['\u2019]/g, '')).reduce(
+    (result, word, index, wordSplit) => {
+      const useHyphen =
+        index &&
+        !(wordSplit[index - 1]!.toLowerCase() === 'v' && /^\d+$/.test(word))
+
+      return result + (useHyphen ? '-' : '') + word.toLowerCase()
+    },
+    '',
+  )
+}
+


nice thank you

jpsains · 2024-09-06T08:43:13Z

...ges/adapters-library/src/adapters/stargate/tests/snapshots/ethereum.withdrawals.farm-v1.json

+          {
+            "address": "0xAf5191B0De278C7286d6C7CC6ab6BB8A73bA2Cd6",
+            "name": "StargateToken",
+            "symbol": "STG",
+            "decimals": 18,
+            "type": "underlying",
+            "blockNumber": 20661441,
+            "balanceRaw": "29168188478059884180158n",
+            "price": 0.27636857144740007,
+            "balance": 29168.188478059885
+          }


is this a claimed rewards? If so is the type correct? I think it should it be underlying-claimable?

I thought so too. But every other adapter implementing the getRewardWithdrawals returns them as underlying, so I used this for consistency.

jpsains · 2024-09-06T08:45:13Z

packages/adapters-library/src/adapters/stargate/products/farm-v2/stargateFarmV2Adapter.ts

+            name: rewardToken.name,
+            symbol: rewardToken.symbol,
+            decimals: rewardToken.decimals,
+            type: TokenType.Underlying,


underlying-claimable

As mentioned before. I'm using the same type that other getRewardWithdrawals implementations (curve and convex) use.

Maybe we need to change them all.

bergarces added 6 commits August 28, 2024 15:20

missing chains

7d817c3

added lp staking

7d0137d

changed name of product

c808aa0

add support for different farming strategies

ccbb103

stargate-v2

ab853ff

snapshot

00f9a5f

bergarces force-pushed the MMI-5405-investigate-and-solve-for-missing-product-on-stargate-to-ensure-we-have-full-coverage branch from a1b150c to 00f9a5f Compare August 29, 2024 21:20

bergarces added 7 commits August 30, 2024 14:02

stargate v2 farming

2723aa7

events for lp staking

1304b2e

deposit test case

a1632aa

withdrawal

c034b96

deposits

e62e488

reward withdrawal

ae65fdf

remove comment

f7ec6b6

bergarces commented Sep 2, 2024

View reviewed changes

bergarces marked this pull request as ready for review September 2, 2024 16:05

bergarces requested a review from jpsains as a code owner September 2, 2024 16:05

bergarces commented Sep 2, 2024

View reviewed changes

bergarces added 8 commits September 2, 2024 17:07

Merge branch 'main' into MMI-5405-investigate-and-solve-for-missing-p…

ff84222

…roduct-on-stargate-to-ensure-we-have-full-coverage

Merge branch 'main' into MMI-5405-investigate-and-solve-for-missing-p…

3497d5d

…roduct-on-stargate-to-ensure-we-have-full-coverage

better test keys

dd6bbc7

fixed names

7bacac6

move v2 pool

ba9d8ab

move farm adapter

4895733

move more metadata

41cc026

fixed tests

1346403

bergarces commented Sep 5, 2024

View reviewed changes

linting

d737fbd

jpsains reviewed Sep 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MMI-5405-investigate-and-solve-for-missing-product-on-stargate-to-ensure-we-have-full-coverage #304

MMI-5405-investigate-and-solve-for-missing-product-on-stargate-to-ensure-we-have-full-coverage #304

bergarces commented Aug 28, 2024 •

edited

Loading

bergarces Sep 2, 2024

jpsains Sep 6, 2024

bergarces Sep 6, 2024

bergarces Sep 2, 2024

bergarces Sep 5, 2024

jpsains Sep 6, 2024

bergarces Sep 5, 2024

jpsains Sep 6, 2024

sonarcloud bot commented Sep 5, 2024

jpsains Sep 6, 2024 •

edited

Loading

bergarces Sep 6, 2024

jpsains Sep 6, 2024

jpsains Sep 6, 2024

jpsains Sep 6, 2024

bergarces Sep 6, 2024

jpsains Sep 6, 2024

bergarces Sep 6, 2024

jpsains Sep 6, 2024

jpsains Sep 6, 2024

bergarces Sep 6, 2024

jpsains Sep 6, 2024

jpsains Sep 6, 2024

bergarces Sep 6, 2024

jpsains Sep 6, 2024

bergarces Sep 6, 2024

MMI-5405-investigate-and-solve-for-missing-product-on-stargate-to-ensure-we-have-full-coverage #304

Are you sure you want to change the base?

MMI-5405-investigate-and-solve-for-missing-product-on-stargate-to-ensure-we-have-full-coverage #304

Conversation

bergarces commented Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 5, 2024

Quality Gate failed

jpsains Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergarces commented Aug 28, 2024 •

edited

Loading

jpsains Sep 6, 2024 •

edited

Loading