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.

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

Fix : add requirements, consequences and side effects natspec - issue 26 #330

Merged
merged 10 commits into from
Nov 2, 2023

Conversation

Jean-Grimal
Copy link
Contributor

@Jean-Grimal Jean-Grimal commented Oct 25, 2023

Fixes https://github.com/cantinasec/review-morpho-blue-1/issues/26

I didn't add such comments on AaveV3, AaveV2 and AaveV3Optimizer bundlers withdraw functions because receiver will be removed

@Jean-Grimal Jean-Grimal requested review from a team, Rubilmax and MerlinEgalite and removed request for a team October 25, 2023 13:57
src/ERC4626Bundler.sol Outdated Show resolved Hide resolved
src/MorphoBundler.sol Outdated Show resolved Hide resolved
src/MorphoBundler.sol Outdated Show resolved Hide resolved
@Rubilmax
Copy link
Collaborator

Fixes cantinasec/review-morpho-blue-1#26

I didn't add such comments on AaveV3, AaveV2 and AaveV3Optimizer bundlers withdraw functions because receiver will be removed

I don't get it: because receiver parameters are expected to be removed, it is actually expected to add the comment to not forget about transferring received funds?

@Jean-Grimal
Copy link
Contributor Author

Fixes cantinasec/review-morpho-blue-1#26
I didn't add such comments on AaveV3, AaveV2 and AaveV3Optimizer bundlers withdraw functions because receiver will be removed

I don't get it: because receiver parameters are expected to be removed, it is actually expected to add the comment to not forget about transferring received funds?

Yes I was mistaken. But actually (regarding migration bundlers), withdrawn assets are always supplied on Blue, so should the natspec be the same as other bundler (i.e. Withdrawn assets must be transferred from the Bundler to a receiver) Or a specific one like Withdrawn assets must be supplied on Blue (or no natspec for this) ?

@Rubilmax Rubilmax changed the base branch from fix/issue-17 to fix/issue-27 October 26, 2023 09:59
src/TransferBundler.sol Outdated Show resolved Hide resolved
@Rubilmax Rubilmax self-assigned this Oct 26, 2023
Base automatically changed from fix/issue-27 to review-cantina October 30, 2023 17:03
Copy link

openzeppelin-code bot commented Nov 1, 2023

Fix : add requirements, consequences and side effects natspec - issue 26

Generated at commit: f4747e3ac6d0096460c138ba040bcdcc02c607fc

🚨 Vulnerabilities Summary

Process Issues Results
Contract Inspector note
low
critical
Total
20
10
1
31
Dependency Checker Total 0

For more details view the full report in OpenZeppelin Code

@MerlinEgalite MerlinEgalite merged commit 79ccaa9 into review-cantina Nov 2, 2023
8 checks passed
@MerlinEgalite MerlinEgalite deleted the fix/issue-26 branch November 2, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants