feat(contracts): ValueRouter #4814
Conversation
…#4729) ### Description - implements `_outbound` and `_inbound` internal amount transforms for use in scaling warp routes - simplify `HypNativeScaled` implementation ### Backward compatibility Yes ### Testing Existing HypNative Scaled Unit Tests
…4673) ### Description - fixes misuse of aggregation hook funds for relaying messages by making sure msg.value is adequate and refunding if excess. ### Drive-by changes - None ### Related issues - related to #3437 ### Backward compatibility No, needs new deployments of aggregationHooks ### Testing Unit
🦋 Changeset detectedLatest commit: 13e79af The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| gasLimit(_metadata, 0), | ||
| refundAddress(_metadata, msg.sender), |
There was a problem hiding this comment.
I dont understand the 0 and msg.sender here
this will also memcopy for every field iiuc
There was a problem hiding this comment.
these are just default values in case the specific part of the metadata is null
true but memcopy is cheap
There was a problem hiding this comment.
0 gas limit and msg.sender for refund address as defaults does not make sense to me
There was a problem hiding this comment.
what would you have in their place?
aroralanuk
requested review from
paulbalaji,
Mo-Hussain and
jmrossy
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4814 +/- ##
==========================================
+ Coverage 77.53% 77.58% +0.05%
==========================================
Files 103 104 +1
Lines 2110 2124 +14
Branches 190 190
==========================================
+ Hits 1636 1648 +12
- Misses 453 455 +2
Partials 21 21
|
| bytes calldata emptyBytes; | ||
| assembly { | ||
| emptyBytes.length := 0 | ||
| emptyBytes.offset := 0 | ||
| } |
There was a problem hiding this comment.
we do not use inline assembly anywhere else, please do not introduce it here.
There was a problem hiding this comment.
need a way to do bytes memory to calldata
| @@ -0,0 +1,84 @@ | |||
| // SPDX-License-Identifier: MIT OR Apache-2.0 | |||
| pragma solidity >=0.8.0; | |||
Check notice
Code scanning / Olympix Integrated Security
Using an unbounded pragma for Solidity version may be unsafe if future versions introduce breaking changes. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unbounded-pragma Low
| * @notice This contract facilitates the transfer of value between chains using value transfer hooks | ||
| */ | ||
| contract HypNativeCollateral is HypNative { | ||
| constructor(address _mailbox) HypNative(_mailbox) {} |
Check notice
Code scanning / Olympix Integrated Security
Test functions fail to thoroughly test all aspects of contract constructors, potentially missing critical initialization issues. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/incomplete-constructor-tests Low
| * @notice This contract facilitates the transfer of value between chains using value transfer hooks | ||
| */ | ||
| contract HypNativeCollateral is HypNative { | ||
| constructor(address _mailbox) HypNative(_mailbox) {} |
Check notice
Code scanning / Olympix Integrated Security
Parameters passed to a constructor that are not validated for correct values may lead to contract creation in an undesired state. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/no-parameter-validation-in-constructor Low
…d interface for sending value hook/ism agnostic
Description
transferRemoteinterface to send msg value from A to B agnostic or the hook/ism needed to do soDrive-by changes
Related issues
Backward compatibility
Yes
Testing
Unit