Fix contract functions #516
Conversation
|
Requesting retroactive review |
| await expect(sdk.silo.mowMultiple(account, [nonWhitelistedToken])).rejects.toThrow(`${nonWhitelistedToken.symbol} is not whitelisted`); | ||
| }); | ||
|
|
||
| it.skip("warns when single token provided", async () => { |
There was a problem hiding this comment.
I'll re-enable this and following tests later on when doing more testing against a local fork with the silo v3 code deployed
| @@ -427,7 +454,7 @@ export class Silo { | |||
| */ | |||
| async getSeeds(_account?: string) { | |||
| const account = await Silo.sdk.getAccount(_account); | |||
| return Silo.sdk.contracts.beanstalk.balanceOfSeeds(account).then((v) => Silo.sdk.tokens.SEEDS.fromBlockchain(v)); | |||
| return Silo.sdk.contracts.beanstalk.balanceOfLegacySeeds(account).then((v) => Silo.sdk.tokens.SEEDS.fromBlockchain(v)); | |||
There was a problem hiding this comment.
TO DISCUSS: This now reads from the legacy balanceOfSeeds storage slot:
function balanceOfSeeds(address account) internal view returns (uint256) {
AppStorage storage s = LibAppStorage.diamondStorage();
return s.a[account].s.seeds;
}
Seeds don't really exist now. A Seed previously granted 1 / 10_000 Stalk per Season. Now each token has a configuration value at s.ss[token].stalkEarnedPerSeason which defines the amount of STALK earned per BDV per Season. We should discuss UX implications
| @@ -456,18 +483,25 @@ export class Silo { | |||
| */ | |||
| async getPlantableSeeds(_account?: string) { | |||
| const account = await Silo.sdk.getAccount(_account); | |||
| // TODO: this is wrong | |||
| return Silo.sdk.contracts.beanstalk.balanceOfEarnedSeeds(account).then((v) => Silo.sdk.tokens.SEEDS.fromBlockchain(v)); | |||
| throw new Error("Not implemented"); | |||
There was a problem hiding this comment.
I guess this is more like "Plantable BDV" now? Since I have some BDV in Beans that I can plant which don't earn Stalk until I plant. For now, throwing an error here and can ship a separate PR to discuss.
There was a problem hiding this comment.
not sure why this wouldn't be Plantable Seeds
| @@ -319,9 +321,12 @@ export class Tokens { | |||
|
|
|||
| ////////// Groups ////////// | |||
|
|
|||
| const siloWhitelist = [this.BEAN, this.BEAN_CRV3_LP, this.UNRIPE_BEAN, this.UNRIPE_BEAN_CRV3]; | |||
| this.siloWhitelist = new Set(siloWhitelist); | |||
| this.siloWhitelistAddresses = siloWhitelist.map((t) => t.address); | |||
There was a problem hiding this comment.
Found myself doing this a lot so I set it up here to optimize
| Withdraw.sdk.debug("silo.withdraw(): withdrawDeposit()", { address: token.address, season: seasons[0], amount: amounts[0] }); | ||
| contractCall = Withdraw.sdk.contracts.beanstalk.withdrawDeposit(token.address, seasons[0], amounts[0]); | ||
| contractCall = Withdraw.sdk.contracts.beanstalk.withdrawDeposit(token.address, seasons[0], amounts[0], toMode); | ||
| } else { | ||
| Withdraw.sdk.debug("silo.withdraw(): withdrawDeposits()", { address: token.address, seasons: seasons, amounts: amounts }); | ||
| contractCall = Withdraw.sdk.contracts.beanstalk.withdrawDeposits(token.address, seasons, amounts); | ||
| contractCall = Withdraw.sdk.contracts.beanstalk.withdrawDeposits(token.address, seasons, amounts, toMode); |
There was a problem hiding this comment.
Shouldn't these pass stem instead of season? Might just be a variable name change
There was a problem hiding this comment.
I'll follow up with a separate PR to do an exhaustive season -> stem renaming where applicable, figured it'd be too much for the scope here; this simply gets contract funcs back up to speed
| @@ -319,9 +321,12 @@ export class Tokens { | |||
|
|
|||
| ////////// Groups ////////// | |||
|
|
|||
| const siloWhitelist = [this.BEAN, this.BEAN_CRV3_LP, this.UNRIPE_BEAN, this.UNRIPE_BEAN_CRV3]; | |||
| this.siloWhitelist = new Set(siloWhitelist); | |||
| this.siloWhitelistAddresses = siloWhitelist.map((t) => t.address); | |||
There was a problem hiding this comment.
We don't need this, an array of addresses can easily been derived from a Set of tokens where needed.
const addr = [...this.siloWhitelist].map(t => t.address)
There was a problem hiding this comment.
Just now saw your other comment about this. You used it in two places, one of which didn't need to be an array. Not a big deal either way
There was a problem hiding this comment.
I'll take a look at this again and we can revert if it's not used very often
There was a problem hiding this comment.
Constructor parameters changed, needs to be updated here as well. Same for WithdrawDeposits (plural)
ui/src/lib/Txn/FarmSteps/silo/WithdrawFarmStep.ts, 62 and 70
There was a problem hiding this comment.
I'll get to updating the UI's steps to explicitly set this value, but for now they fall back to the default INTERNAL value since that's set on the constructor. Lmk if you think this is undesirable.
After this PR,
yarn all:buildis running again. The UI is not included in that build step. A separate PR will follow to fix explicit UI issues.mowAndMigratefunctiontoModeparameter associated with zero withdraw upgradeNote that
MowFarmStepin the UI still calls .update directly and due to a typing issue this is compiling. A separate PR will ship to resolve (possibly disable) this.