Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(contracts): ValueRouter #4814
base: main
Are you sure you want to change the base?
feat(contracts): ValueRouter #4814
Changes from 12 commits
3e2f9e3
a51b288
7215577
4556e24
99647fe
6876905
2514043
670912b
3fec6a0
1e54bf6
d8d4d2f
e302e9c
ecd1e90
8fb963c
bee6d22
bab9928
139ba55
9fcc4d9
41ff083
9c38a7a
5c1f2e8
ec225b4
d996b3a
4f04099
7cf2d75
b541129
13e79af
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont understand the 0 and msg.sender here
this will also memcopy for every field iiuc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 gas limit and msg.sender for refund address as defaults does not make sense to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would you have in their place?
Check warning on line 108 in solidity/contracts/token/HypNative.sol
Codecov / codecov/patch
solidity/contracts/token/HypNative.sol#L107-L108
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do not use inline assembly anywhere else, please do not introduce it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a way to do bytes memory to calldata
Check warning on line 148 in solidity/contracts/token/HypNative.sol
Codecov / codecov/patch
solidity/contracts/token/HypNative.sol#L148
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