-
Notifications
You must be signed in to change notification settings - Fork 182
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
🧹 simplify read examples #1035
base: main
Are you sure you want to change the base?
🧹 simplify read examples #1035
Conversation
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat are unpopular packages?This package is not very popular. Unpopular packages may have less maintenance and contain other problems. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
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.
Just some small stuff
8907e13
to
f3821b4
Compare
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
f3821b4
to
074a7b9
Compare
packages/oapp-evm-upgradeable/artifacts/ExecutorOptions.sol/ExecutorOptions.json
Show resolved
Hide resolved
8c50dc4
to
972871a
Compare
Fixed pnpm-lock.yaml. Running through CI pipeline. |
This PR is awesome. when can we have this in main ? |
ac7d8db
to
c146a6f
Compare
uint32 _readChannel | ||
) OAppRead(_endpoint, _delegate) Ownable(_delegate) { | ||
READ_CHANNEL = _readChannel; | ||
_setPeer(_readChannel, AddressCast.toBytes32(address(this))); |
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.
not review but why doesnt OAppRead
set self peer? in lzRead we always self peer dont we?
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.
OAppRead shouldn't override setPeer since than if you used it for just messaging you wouldn't be able to.
addressToBytes32(address(bOApp)), | ||
0, | ||
address(0x0), | ||
abi.encode(a + b) // The sum of a and b |
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.
fuzz here because a+b
could overflow
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.
Think this is unnecessary since this is an example contract, there will likely be edge cases that cause a + b to overflow.
examples/view-pure-read/test/foundry/ReadViewOrPureAndCompute.t.sol
Outdated
Show resolved
Hide resolved
1ecfa5c
to
de2ed56
Compare
No description provided.