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

fix: N01-N14: Address Noncritical Issues #728

Merged
merged 28 commits into from
Nov 19, 2024
Merged

fix: N01-N14: Address Noncritical Issues #728

merged 28 commits into from
Nov 19, 2024

Conversation

bmzig
Copy link
Contributor

@bmzig bmzig commented Nov 7, 2024

This updates parts of the L3 code which were deemed as non-critical issues.

bmzig added 17 commits November 7, 2024 08:01
@bmzig
fix: remove duplicated logic
1ecbb3f 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
improve: allow the removal of adapters
b1f17b4 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
chore: update/fix comments
5298182 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
chore: remove unused SafeERC20 import
507dba7 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
nit: explicitly define visibility for chainAdapters
5971acc 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
nit: consistent comments
2fbfc88 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
improve: index event fields
2ad851d 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
chore: remove legacy comments
edfadf4 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
custom gas token comment
75b2974 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
Merge branch '1124oz/n02' into bz/ozNonCriticals
b621adf
@bmzig
Merge branch '1124oz/n04' into bz/ozNonCriticals
702f21e
@bmzig
Merge branch '1124oz/n05' into bz/ozNonCriticals
98100c3
@bmzig
Merge branch '1124oz/n07' into bz/ozNonCriticals
467a207
@bmzig
Merge branch '1124oz/n08' into bz/ozNonCriticals
30b1ecb
@bmzig
Merge branch '1124oz/n09' into bz/ozNonCriticals
e13dd1f
@bmzig
Merge branch '1124oz/n13' into bz/ozNonCriticals
06bd68d
@bmzig
use consistent pragmas
c335be2 
Signed-off-by: bennett <bennett@umaproject.org>
bmzig added 3 commits November 8, 2024 09:34
@bmzig
N-03: Unused Struct
36b4869 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
N-11: File and Contract Names Mismatch
3df9450 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
N-12: Constant Could be Used to Denote ETH on L2s
813bf95 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig bmzig changed the title fix: address noncritical issues fix: N01-N14: Address Noncritical Issues Nov 8, 2024
bmzig added 2 commits November 8, 2024 12:22
@bmzig
N-14 Incorrect Interface Implementation
88ae26a 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
N-10 Naming Suggestions
03a0d1a 
Signed-off-by: bennett <bennett@umaproject.org>
pxrl
pxrl reviewed Nov 14, 2024
View reviewed changes
Copy link
Contributor

@pxrl pxrl left a comment

Choose a reason for hiding this comment

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

LGTM - I was able to reconcile all changes to the notes raised by the audit. I have a couple of residual queries to align on cc @mrice32 @nicholaspai

contracts/Arbitrum_SpokePool.sol Show resolved Hide resolved
contracts/Arbitrum_SpokePool.sol Outdated Show resolved Hide resolved
contracts/chain-adapters/l2/Arbitrum_WithdrawalHelper.sol Show resolved Hide resolved
contracts/chain-adapters/l2/Arbitrum_WithdrawalHelper.sol Outdated
@@ -1,7 +1,4 @@
// SPDX-License-Identifier: BUSL-1.1

// Arbitrum only supports v0.8.19
// See https://docs.arbitrum.io/for-devs/concepts/differences-between-arbitrum-ethereum/solidity-support#differences-from-solidity-on-ethereum
pragma solidity ^0.8.19;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above RE potentially updating this pragma in light of subsequent ArbOS updates removing lifting a historical constraint.

contracts/chain-adapters/l2/Ovm_WithdrawalHelper.sol Outdated Show resolved Hide resolved
contracts/chain-adapters/ZkStack_CustomGasToken_Adapter.sol Show resolved Hide resolved
contracts/chain-adapters/ZkStack_CustomGasToken_Adapter.sol Outdated Show resolved Hide resolved
contracts/chain-adapters/ZkStack_CustomGasToken_Adapter.sol Show resolved Hide resolved
contracts/erc7683/ERC7683OrderDepositor.sol Show resolved Hide resolved
contracts/chain-adapters/l2/Ovm_WithdrawalHelper.sol Show resolved Hide resolved
@bmzig bmzig marked this pull request as ready for review November 14, 2024 14:33
@bmzig bmzig requested review from nicholaspai and mrice32 November 14, 2024 14:33
@bmzig
Merge master and update hardhat.config.ts
12c4d53 
Signed-off-by: bennett <bennett@umaproject.org>
@nicholaspai
Copy link
Member

Can you comment on which lines are related to which issues? would make it faster to review

@bmzig
Merge 'master' and make pragmas consistent
b6cae1c 
Signed-off-by: bennett <bennett@umaproject.org>
bmzig
bmzig commented Nov 18, 2024
View reviewed changes
contracts/erc7683/ERC7683Permit2Lib.sol
Copy link
Contributor Author

Choose a reason for hiding this comment

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

N-11

@bmzig
delete duplicate file
79b75ae 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
Copy link
Contributor Author

bmzig commented Nov 18, 2024

Can you comment on which lines are related to which issues? would make it faster to review

Paul made comments on most of them, and I covered the gaps. There should be comments for 1-14

nicholaspai
nicholaspai reviewed Nov 18, 2024
View reviewed changes
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

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

https://github.com/across-protocol/contracts/pull/728/files#r1844570282
https://github.com/across-protocol/contracts/pull/728/files#r1846744574
https://github.com/across-protocol/contracts/pull/728/files#r1846747834

A few outstanding comments then we'll be good, good work!

bmzig added 2 commits November 18, 2024 14:14
@bmzig
Merge branch 'master' into 1124oz/nonCriticals
8ee3938 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
consolidate updateAdapter and change arbitrum spoke pool pragma
8e43769 
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig bmzig merged commit 7f9ebb3 into master Nov 19, 2024
9 checks passed
@bmzig bmzig deleted the 1124oz/nonCriticals branch November 19, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pxrl pxrl pxrl left review comments

@nicholaspai nicholaspai nicholaspai approved these changes

@mrice32 mrice32 Awaiting requested review from mrice32 mrice32 is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@bmzig @nicholaspai @pxrl