Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: replace native token in lending contract #2276

Merged
merged 1 commit into from
Sep 13, 2023
Merged

refactor: replace native token in lending contract #2276

merged 1 commit into from
Sep 13, 2023

Conversation

LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Sep 13, 2023 •
edited
Loading

Fixes #2275.

  • Moves simulators into a separate simulator folder for easier access from other testfiles.
  • Refactors lending tests to be more similar to the token tests running "invariant" checks on afterEach
  • Burns borrowed asset when repaying

Since the lending tests is essentially rewritten might be easier to not check the diff but look at it as a fully new file.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

@LHerskind LHerskind force-pushed the lh/lending_replace_native_token branch 2 times, most recently from 23aa8fd to d8ca053 Compare September 13, 2023 15:13
Maddiaa0
Maddiaa0 approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

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

looks great tbh, really clean

yarn-project/noir-contracts/src/contracts/lending_contract/src/interfaces.nr
@@ -36,36 +36,44 @@ impl Token {
Self { address }
}

fn transfer_pub(self: Self, context: PublicContext, to: Field, amount: Field) {
fn transfer_public(self: Self, context: PublicContext, from: Field, to: Field, amount: Field, nonce: Field) {
Copy link
Member

Choose a reason for hiding this comment

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

is this generated of by hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hand generated some time back and just altered here.

yarn-project/end-to-end/src/e2e_lending_contract.test.ts

describe('e2e_lending_contract', () => {
jest.setTimeout(100_000);
Copy link
Member

Choose a reason for hiding this comment

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

:(

yarn-project/end-to-end/src/e2e_lending_contract.test.ts
logger(`CDP deployed at ${receipt.contractAddress}`);
lendingContract = await LendingContract.at(receipt.contractAddress!, wallet);
}

await waitForSuccess(collateralAsset.methods._initialize(accounts[0]).send());
Copy link
Member

Choose a reason for hiding this comment

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

awesome pattern, (could also call send in the function to clean it up even more! although send here is nice an explicit)

@LHerskind LHerskind force-pushed the lh/lending_replace_native_token branch from d8ca053 to 464f39b Compare September 13, 2023 22:02
@LHerskind LHerskind merged commit c46b3c8 into master Sep 13, 2023
87 checks passed
@LHerskind LHerskind deleted the lh/lending_replace_native_token branch September 13, 2023 22:25
@AztecBot AztecBot mentioned this pull request Sep 13, 2023
Merged
PhilWindle pushed a commit that referenced this pull request Sep 14, 2023
@AztecBot
chore(master): Release 0.7.1 (#2268)
4d3e341 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-packages: 0.7.1</summary>

##
[0.7.1](aztec-packages-v0.7.0...aztec-packages-v0.7.1)
(2023-09-14)


### Features

* Build system handles dynamic deps first class.
([#2283](#2283))
([f66077a](f66077a))
* Build_manifest default tweaks.
([#2287](#2287))
([c8a5cfb](c8a5cfb))
* **build:** Build multi-architecture docker images for aztec-sandbox
([#2305](#2305))
([8ee61b8](8ee61b8))
* Cli "unbox" command
([#2029](#2029))
([26ab88f](26ab88f))
* Creating an SMT verification module
([#1932](#1932))
([4642b61](4642b61))
* Token standard
([#2069](#2069))
([5e8fbf2](5e8fbf2))


### Bug Fixes

* Ensure_note_hash_exists
([#2256](#2256))
([271b060](271b060))
* Msgpack stack blowups on schema gen
([#2259](#2259))
([1afc566](1afc566))
* Noir bootstrap
([#2274](#2274))
([f85db49](f85db49))
* Workaround sequencer timeout
([#2269](#2269))
([9fc3f3d](9fc3f3d))


### Miscellaneous

* Bump nargo to 0.11.1-aztec.0
([#2298](#2298))
([8b76a12](8b76a12))
* **ci:** Mirror Aztec-nr
([#2270](#2270))
([c57f027](c57f027))
* **circuits:** Base rollup cbind msgpack
([#2263](#2263))
([0d4c707](0d4c707))
* **circuits:** Clean up of some superfluous header includes
([#2302](#2302))
([5e53345](5e53345))
* **circuits:** Removing assertMemberLength on Tuple objects
([#2296](#2296))
([0247b85](0247b85))
* Consolidate mirror repos on a nightly schedule
([#1994](#1994))
([1a586c4](1a586c4))
* **docs:** Rename to aztec.nr
([#1943](#1943))
([a91db48](a91db48))
* Move barretenberg to top of repo. Make circuits build off barretenberg
build.
([#2221](#2221))
([404ec34](404ec34))
* Replace native token in lending contract
([#2276](#2276))
([c46b3c8](c46b3c8))
* **subrepo:** Push aztec-nr, update default branches
([#2300](#2300))
([80c9b77](80c9b77))
* Updated `acvm_js`
([#2272](#2272))
([9f1a3a5](9f1a3a5))
</details>

<details><summary>barretenberg.js: 0.7.1</summary>

##
[0.7.1](barretenberg.js-v0.7.0...barretenberg.js-v0.7.1)
(2023-09-14)


### Miscellaneous

* Move barretenberg to top of repo. Make circuits build off barretenberg
build.
([#2221](#2221))
([404ec34](404ec34))
</details>

<details><summary>barretenberg: 0.7.1</summary>

##
[0.7.1](barretenberg-v0.7.0...barretenberg-v0.7.1)
(2023-09-14)


### Miscellaneous

* Move barretenberg to top of repo. Make circuits build off barretenberg
build.
([#2221](#2221))
([404ec34](404ec34))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Maddiaa0 Maddiaa0 Maddiaa0 approved these changes

@rahul-kothari rahul-kothari Awaiting requested review from rahul-kothari

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace native_token with token in lending
2 participants
@LHerskind @Maddiaa0