hifi-finance / hifi

Monorepo implementing the Hifi fixed-rate, fixed-term lending protocol
https://app.hifi.finance
Other
105 stars 15 forks source link

feat(proxy-target): accept eip2612 signatures #77

Closed surbhiaudichya closed 2 years ago

PaulRBerg commented 2 years ago

Thanks for this PR, @surbhiaudichya. Before I can review it though, you will have to resolve the git conflicts. You can do that by manually removing the "src/types" directory and re-generating the types via the generate:types script. The reason why types don't use the .d.ts file extension anymore is that the latest version of TypeChain switched to pure .ts extensions in their type generation algorithm.

Also, before this can be used at all, we'd have to switch from @paulrberg/contracts to @prb/contracts - since in v3.8.0 I changed the name of the package. I will handle this part myself.

surbhiaudichya commented 2 years ago

@paulrberg I re-generating the types via thegenerate:types script for the latest version of TypeChain.

PaulRBerg commented 2 years ago

Yes but you also have to resolve the git conflicts before I can review this.

surbhiaudichya commented 2 years ago

@paulrberg branch has no conflicts with the base branch now but some of the checks are still failing, I am looking into it.

PaulRBerg commented 2 years ago

@surbhiaudichya Checks are failing because of the yarn install --immutable command. The immutable flag ensures that the yarn.lock file does not get modified in CI.

PaulRBerg commented 2 years ago

Surbhi, I don't think you rebased properly. I've just seen this commit:

https://github.com/hifi-finance/hifi/pull/77/commits/6cc4afde1e3bae6e9b49e036f081c0f068e06012

But I've already done that 10 days ago.

I think that at this stage it would be easier if you started from scratch from the main branch, created a new PR and closed the current PR.

surbhiaudichya commented 2 years ago

@paulrberg Sure, I can do that. That commit was for the task package the function call to "supplyUnderlying" wasn’t changed to "depositUnderlying" in addLiquidty.ts