-
Notifications
You must be signed in to change notification settings - Fork 48
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
Error when importing @prb/math packages when installed via npm #646
Comments
Thanks for the detailed report @waynehoover. I'm sorry to hear you bumped into all these issues. This may be related to this Hardhat bug: I agree that Hardhat users should not have to install third-party dependencies via Foundry. It's bad DX. Hopefully once Hardhat fixes the bug above, you won't have to do this anymore. Side note: we are considering to move away from remappings to Node.js deps. |
hey @waynehoover! It turns out that there was another bug in Foundry that was getting in the way of making the remappings work: They fixed it in the meantime - if you run |
If I run This is because when @prb/math is imported here: https://github.com/sablier-labs/v2-core/blob/main/src/types/Math.sol#L10 it doesn't include the parent So, there are a couple of options here I see:
|
Thank you, @waynehoover. The extra details are helpful. Using We will likely switch to using npm packages everywhere in the long term since it seems like the most robust solution. I wouldn't want to mess around with the location of the contract files at build time since that would add some maintenance overhead. Similarly, copying over dependencies would require us to keep our copy of PRBMath in sync with the actual, latest PRBMath, and it would also be a bit self-defeating. |
As shown by this conversation, this issue with the I have marked this as "priority0". @andreivladbrg, it would be helpful if you could pick up this task during your next sprint; basically, the solution is to swap all import paths to contain |
) * fix: address #646 * chore: update remapping in slither config --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com>
) * fix: address #646 * chore: update remapping in slither config --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com>
Hey @waynehoover, glad to let you know that we just shipped v1.0.2 of the Let us know if it works! |
* docs: delete wikis * test: polish precompiles tests build: add branch for solady in ".gitmodules" * docs: polish deployments section * docs: polish security program * docs: polish license section * chore: remove Codecov token * refactor: update gas snapshot * build: change version to 1.0.0-rc.0 chore: update "repository" in "package.json" * refactor: stringify create2 salts * refactor: rename deployer to broadcaster feat: use $ETH_FROM as broadcaster * chore: fix rpc url for gnosis chain * docs: refine license section * test: update createSelectFork * chore: use solhint-community build: add "solhint-community" node.js dep build: remove "solhint" node.js * build: bump solady * build: release v1.0.0 * test: remove duplicate bounds * docs: mention audits in the bug bounty * chore: say token streaming * test: rename "basic" to "concrete" * test: order imports correctly * perf: use vars.sablierAddress Closes #607 * perf: define "vars.asset" as "address" Closes #608 * fix: data URI scheme test: update hard-coded values test: use LibString to remove data URI prefix Closes #616 * test: update Precompiles bytecodes * docs: roll v1.0.1 docs: add CHANGELOG * docs: update recommended install branch * chore: add missing modifier in test_SetFlashFee test * Add Contribution info in README.md * test: convert 'flashFee' integration tests to unit tests * test: convert 'setFlashFee' integration tests to unit tests * test: add 'Comptroller' setup for concrete unit test * test: update 'flashFee' unit-test to inherit from Comptroller * test: update 'setFlashFee' unit-test to inherit from Comptroller * style: run prettier on README * test: deploy correctly precompiled comptroller * feat: deploy core 2 * test: change default profile value * Fix error when importing @prb/math packages when installed via npm (#648) * fix: address #646 * chore: update remapping in slither config --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> * test: fix string parameter * docs: roll v1.0.2 * docs: update recommended remapping * docs: add "Architecture" in README closes #551 * perf: dry "isCold" and "isWarm" Closes #610 * docs: fix nitpicks Closes #611 * test: use "given" keyword in branching trees (#656) * refactor: using Given keyword in branching Trees (#641) * Using Given keyword on the branching trees (cherry picked from commit d365705) * test: use "it" for test nodes --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> test: rename to "givenCallerAdmin" test: use given keyword in the integration branching trees test: use given keyword for modifiers for fuzz tests test: use given keyword in test contracts and brancing trees test: refactor test function names to use Given and When test: refactor test function names to use Given and When refactor: reverting in dev comments refactor: rename test function names to cleaner form test: polish branching trees test: improve "mapSymbol" tree test: improve "safeAssetDecimals" tree test: improve "safeAssetSymbol" tree * refactor: givenWithdrawableAmountNotZero modifier name * test: use "when" for time-related branches * test: address feedback in tree wording branch * test: polish branching trees * fix: remove duplicated branch in createWithMilestones.tree --------- Co-authored-by: Shub <shubhamy2015@gmail.com> Co-authored-by: andreivladbrg <andreivladbrg@gmail.com> Co-authored-by: Alexander González <alexfertel97@gmail.com> * docs: mention audits * docs: add Discord badge * test: update comment * fix: fix incorrect spec in isStream.tree (#677) * test: use "given" instead of "when" (#678) * test: use "given" instead of "when" * test: correct "given" branches --------- Co-authored-by: andreivladbrg <andreivladbrg@gmail.com> * ci: cache testing contracts (#681) * Fix Github workflow build cache * ci: add "out" dir in github cache * Update .github/workflows/ci.yml * Update .github/workflows/ci.yml * Update .github/workflows/ci-deep.yml --------- Co-authored-by: Lumyo <lumyo@proton.me> Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> * feat: add transferrable bool field for stream NFT * feat: override _beforeTokenTransfer for Stream NFT and test cases * test: Add test for NFT transfer functionality * test: add more testing and resolve #669 * perf: dry "_beforeTokenTransfer" refactor: remove "canStreamTransfer" refactor: remove double "r" in "transferrable" refactor: simplify implementation test: allow non-transferable streams in invariant tests test: dry tests for "isTransferable" test: improve function and test names * test: add a new branch to burn.tree * test: polish burn tests * test: update precompiles * feat: implement _afterTokenTransfer to emit an event * test: expect MetadataUpdate event to be emitted * test: update Precompiles bytecode * test: transferFrom function * chore: add commented parameters in ERC721 hooks * docs: improve writing in NatSpec test: fix typos * build: upgrade solidity version to 0.8.21 (#688) * build: upgrade solidity version to 0.8.21 * build: bump the pragma back to >=0.8.19 * build: show unproved and unsupported SMTChecker * refactor: update gas snapshot --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> * refactor: change the streamed amount to total amount in NFT SVG (#692) * refactor: change the streamed card from NFT SVG with total build: update shell scripts accordingly test: update tests accordingly * chore: say "total amount" in comments for total card * refactor: change the "Total" from NFT SVG with "Amount" build: update shell script accordingly test: update tests accordingly * chore: fix typo in shell script * refactor: dry-fy withdraw function refactor: dry-fy renounce function * build: remove OZ from peer dependencies * docs: roll 1.1.0 (#693) * docs: roll 1.1.0 * docs: update changelog --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> * chore: fix typo in DataTypes * docs: correct github hyperlink docs: order change logs chronologically * docs: update changelog * build: switch to "solhint" chore: disable Solhint rules * refactor: capitalize immutable variables (#700) * refactor: capitalize immutable variables in NoDelegateCall build: remove "immutable-vars-naming" from solhint file * test: make asset and holder immutable in fork tests test: capitalize constans in fork tests * docs: polish background description * chore: remove "cbor_metadata" * perf: optimize withdraw function * Remove the ability to cancel for recipients (#710) * feat: remove ability to cancel for recipient feat: remove sender's hook test: update tests accordingly * feat: emit asset in cancel and withdraw events refactor: remove recipient from cancel event test: update tests accordingly test: update precompiles bytecode * chore: update gas snapshot * test: update Precompiles bytecode * docs: include recipient disable to cancel in changelog * refactor: add "recipient" in cancel event docs: refine explanatory comments refactor: reorder parameters in events * test: update Precompiles bytecode * chore: add explantory comment --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> * test: add DeployOptimized utils contracts (#712) * test: add DeployOptimized utils contracts test: move and rename functions from Base_Test * docs: fix typo in comments * test: rename deploy optimized functions --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> * style: fix Prettier formatting issue --------- Co-authored-by: Paul Razvan Berg <paul.razvan.berg@gmail.com> Co-authored-by: andreivladbrg <andreivladbrg@gmail.com> Co-authored-by: Anton Cheng <antonassocareer@gmail.com> Co-authored-by: Prince Allwin <127643894+worksofallwin@users.noreply.github.com> Co-authored-by: PraneshASP <praneshas2000@gmail.com> Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com> Co-authored-by: Shub <shubhamy2015@gmail.com> Co-authored-by: Alexander González <alexfertel97@gmail.com> Co-authored-by: Lumyo <lumyo@proton.me>
After adding
@sablier/core-v2
to my package.json I'm getting this error when I compile my contract:File @prb/math/SD59x18.sol, imported from @sablier/v2-core/src/types/Math.sol, not found.
My contract imports look like this:
package.json:
This is because the contracts when deployed to the @prb/math npm package are in the
src
folder as you can see here: https://www.npmjs.com/package/@prb/math?activeTab=code(the same is true for the @sablier/v2-core repo, but that's okay because I can manually add the src/ directory to imports there)
So I suppose a solution would be to move the contracts out of the src folder when deploying the package to npm, this would be backwards compatible with forge, but not with anyone including @prb/math with npm and referencing it from the src directory.
If there is a way to manually map the src folder in npm (like you can with forge's remappings.txt) that would also work, but I didn't find any solutions like that with a quick google.
-Update-
I was able to get this working using the hardhat-foundry plugin and installing @prb/math the foundry way. But ideally, users should be able to use this repo without foundry.
The text was updated successfully, but these errors were encountered: