Fly 2075/add tests and tasks in foundry#369
Merged
Merged
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
…onding scripts and Makefile updates
…des, including Makefile updates and new script for handling refunds
… including Makefile updates, new helper scripts for fetching Bitcoin transaction data, and a Foundry script for registering PegIn transactions.
…rade, and UpgradeLBC scripts, validating ownership transfer, deployment flow, and upgrade patterns for LiquidityBridgeContract.
…sh-quote, pause-system, refund-user-pegout, and register-pegin functionalities, enhancing modularity and maintainability. Remove obsolete bash scripts.
…ing Bitcoin addresses, and update references in Foundry tasks to use TypeScript instead of JavaScript. Additionally, update ESLint configuration to include new directories.
Luisfc68
requested changes
Nov 12, 2025
…t emission expectations for penalization
… fetching address bytes using ffi
…correct OP_RETURN size in LpRefund tests
…ons, ensuring all fields affect the hash. This includes validations for various fields such as callFee, penaltyFee, and others in both PegIn and PegOut contexts.
… handling. Streamline BTC Mock data generation
… handling expired deposit and expire dates in PegOutQuote validation.
… RegisterPegIn tests
…ternal balance crediting when payment fails
…orrect parameters for LP registrations
…t conditions for old owners
…us in both active and paused states
… contracts to use it for Bitcoin address parsing
Luisfc68
reviewed
Nov 20, 2025
Luisfc68
left a comment
Luisfc68
left a comment
Collaborator
There was a problem hiding this comment.
The PR is almost done, just need a few minor fixes
| console.log("Hash from contract:"); | ||
| console.logBytes32(contractHash); | ||
|
|
||
| // The script uses the same contract method, so hashes should match |
Collaborator
There was a problem hiding this comment.
The script still doesn't have any assertions
| console.log("Hash from contract:"); | ||
| console.logBytes32(contractHash); | ||
|
|
||
| // The script uses the same contract method, so hashes should match |
Collaborator
There was a problem hiding this comment.
In this case is also pending
| * @notice Test for the ChangeOwnerToMultiSig deployment script | ||
| * @dev Tests ownership transfer pattern to multisig | ||
| */ | ||
| contract ChangeOwnerToMultiSigTest is Test { |
Collaborator
There was a problem hiding this comment.
Still pending, you could just try to setProviderStatus with the new owner
|
|
||
| // Register by someone else (not LP) - LP gets penalized | ||
| vm.prank(registerCaller); | ||
| pegInContract.registerPegIn( |
Collaborator
There was a problem hiding this comment.
still needs to verify the events
|
|
||
| // Register should revert | ||
| vm.prank(fullLp); | ||
| vm.expectRevert(); // AmountTooLow from Quotes |
| ); | ||
| } | ||
|
|
||
| // Note: Reentrancy tests would require deploying malicious contracts that attempt |
| } | ||
|
|
||
| function test_HashPegOutQuote_HashesPegOutQuoteProperly() public view { | ||
| // Note: Like PegIn hashing tests, we verify determinism rather than exact hashes |
Collaborator
There was a problem hiding this comment.
Can't you use an approach similar to the one used in the hash quote task to address this issue?
| ); | ||
| } | ||
|
|
||
| function test_RefundPegOut_RevertsIfNullDataScriptHasWrongSize() public { |
Collaborator
There was a problem hiding this comment.
This test is correct and I'd leave it, but my comment was about the case where the null data script is correct in format but the quote hash it represents doesn't match the quote being refunded
Collaborator
There was a problem hiding this comment.
So a copy paste of this test with a diff OP_RETURN should work
Collaborator
|
I'm merging the PR to unblock depending PRs. The fixes can be addressed in a new one as they're minor |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What
The task