Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

feat(bastion-protocol): Add Bastion protocol to Zapper #628

Merged
merged 15 commits into from
Jun 14, 2022
Merged

feat(bastion-protocol): Add Bastion protocol to Zapper #628

merged 15 commits into from
Jun 14, 2022

Conversation

BastionProvider
Copy link
Contributor

@BastionProvider BastionProvider commented Jun 11, 2022

Description

Bastion Lending and Stableswap integration to Zapper

Checklist

  • I have followed the Contributing Guidelines
  • (optional) As a contributor, my Ethereum address/ENS is:
  • (optional) As a contributor, my Twitter handle is:

How to test?

const borrowApy = Math.pow(1 + secondsPerDay * Number(borrowRate), 365) - 1;

// Display Props
const label = underlyingToken.symbol;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use the realmName in the label. The dataProps are not displayed in the UI, only the displayProps.

Copy link
Contributor

@immasandwich immasandwich Jun 13, 2022

Choose a reason for hiding this comment

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

Example:

const label = `${underlyingToken.symbol} in ${realmName}`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did just that, thanks for letting me know.

};

@Injectable()
export class BastionSupplyTokenHelper {
Copy link
Contributor

@immasandwich immasandwich Jun 13, 2022

Choose a reason for hiding this comment

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

Is it possible to avoid the code duplication by using the CompoundSupplyTokenHelper from src/apps/compound/helpers? That helper should work for any Compound fork.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can take inspiration from src/apps/market-xyz/avalanche/market-xyz.supply.token-fetcher.ts, which is a lot like Bastion in that there are multiple isolated lending markets that all operate via separate comptrollers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used CompoundSupplyTokenHelper now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like our ABI is not compatible with compound's, I will fix that first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of the ABI difference, I think it's best to just stick with using our own helper.

};

@Injectable()
export class BastionSwapTokenHelper {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use the CurvePoolTokenHelper from src/apps/curve/helpers? See src/apps/saddle/ethereum/saddle.pool.token-fetcher.ts for an example of a Curve fork that uses those helper classes to avoid code duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't keep track of the volume at the moment so the value is zero for now. Otherwise, using the CurvePoolTokenHelper works fine.

Copy link
Contributor

@immasandwich immasandwich left a comment

Choose a reason for hiding this comment

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

Overall the implementation will work, but you could probably remove a lot of code duplication using the helper classes from the Compound and Curve apps.

One required change would be to add the realmName to the label so that users can differentiate their deposited/borrowed balances across different realms.

Ping me once you're ready for a merge!

@BastionProvider
Copy link
Contributor Author

BastionProvider commented Jun 14, 2022

I think it's good for merging, let me know if you still have any issues.

To sum up the changes:

  • I used the CurvePoolTokenHelper for the stableswap.
  • I didn't use the CompoundSupplyTokenHelper because of ABI differences.
  • I moved the realmName to the label

@immasandwich
Copy link
Contributor

Thank you for your patience ser @BastionProvider

Merging now.

@immasandwich immasandwich merged commit 1031765 into Zapper-fi:main Jun 14, 2022
@SamIam-0x
Copy link
Contributor

Hello @BastionProvider !

Looks like you did not provide an address when submitting your PR. I recognize it may have seemed kind of random that we were asking for a wallet address with a PR submission, but we are actually looking for ways to reward Studio contributors in the future.

Is there an address you’d want to provide? Could even be an empty address, if you do not have an doxxed address

Let me know! Also, if you’d like to privately provide it, feel free to shoot me a TG https://t.me/SamIAm_0x or DM on the Zapper Discord.

Thanks!
Sam

@SamIam-0x
Copy link
Contributor

Gentle ping on the above @BastionProvider ! Let me know

@BastionProvider
Copy link
Contributor Author

BastionProvider commented Aug 23, 2022

Hey, sorry I just saw this. What's your tag on discord? I can't find you on the zapper discord for some reason. I'll DM the address there @SamIam-0x .

@SamIam-0x
Copy link
Contributor

Hey, sorry I just saw this. What's your tag on discord? I can't find you on the zapper discord for some reason. I'll DM the address there @SamIam-0x .

Hey- Discord name is Sam | Zapper#8264

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants