Skip to content
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: Implement renounceMultiple #1061

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from

Conversation

cxp-13
Copy link

@cxp-13 cxp-13 commented Oct 10, 2024

Closes #1052

Added

  • Add renounceMultiple to the contract section

Changed

  • Change the visibility of the renouncing function from external to public

Tests:

  • Test renounceMultiple under specific circumstances

* test: Increase testing of multi renounce in dynamic, linear, and segmented situations.
* docs: Add renounceMultiplet.tree
@smol-ninja smol-ninja changed the title feat: Implement "renounceMultiple"(#1052) feat: Implement "renounceMultiple" Oct 10, 2024
@smol-ninja smol-ninja changed the title feat: Implement "renounceMultiple" feat: Implement renounceMultiple Oct 10, 2024
@smol-ninja
Copy link
Member

smol-ninja commented Oct 10, 2024

Great PR @cxp-13. There are some merge conflicts. Can you please resolve them and update the PR? Thank you.

@cxp-13
Copy link
Author

cxp-13 commented Oct 11, 2024

Great PR @cxp-13. There are some merge conflicts. Can you please resolve them and update the PR? Thank you.

I merged the staging branch on my own branch, and only a few files were in conflict. I fixed them, but why are there so many file modifications when I submit them.

My own branch cxp-13-feat/renouncemultimultiple is created based on the main branch.

@smol-ninja
Copy link
Member

smol-ninja commented Oct 11, 2024

merged the staging branch on my own branch, and only a few files were in conflict. I fixed them, but why are there so many file modifications when I submit them.

I'd recommend to reset your local branch with the staging branch and then cherry-pick your commits.

$ git branch temp
$ git reset --hard origin/staging
$ git cherry-pick your_commit

@cxp-13
Copy link
Author

cxp-13 commented Oct 11, 2024

cxp@CXPfromR9000P:~/solana_learn/v2-core$ forge build
[⠢] Compiling...
[⠒] Compiling 281 files with Solc 0.8.26
Error: 
solc exited with signal: 9 (SIGKILL)
<empty output>

When I first cloned and implemented this feature, the total compiled files were less than 200. At that time, it could be successfully compiled.
Then, I don't know which instruction it is. It will increase to over 280 files and my memory cannot handle compilation. Even if I switch to the main branch and execute git checkout . I executed forget clean again. It is also impossible to revert back to the state when it was first cloned.

@cxp-13
Copy link
Author

cxp-13 commented Oct 11, 2024

merged the staging branch on my own branch, and only a few files were in conflict. I fixed them, but why are there so many file modifications when I submit them.

I'd recommend to reset your local branch with the staging branch and then cherry-pick your commits.

$ git branch temp
$ git reset --hard origin/staging
$ git cherry-pick your_commit

I understand what you mean, but I have already tried merging and submitting. Can we do this again?

@cxp-13
Copy link
Author

cxp-13 commented Oct 11, 2024

cxp@CXPfromR9000P:~/solana_learn/v2-core$ bun run build:optimized
$ FOUNDRY_PROFILE=optimized forge build
[⠒] Compiling...
[⠢] Compiling 110 files with Solc 0.8.26
[⠆] Solc 0.8.26 finished in 100.15s
Compiler run successful!

Great, this method can compile successfully.

@smol-ninja
Copy link
Member

@cxp-13 before we review it, I'd appreciate if you can fix the failing CI. In case you ain't aware of the Bulloak, you can find details about it in the contributing guidelines.

@cxp-13
Copy link
Author

cxp-13 commented Oct 11, 2024

@cxp-13 before we review it, I'd appreciate if you can fix the failing CI. In case you ain't aware of the Bulloak, you can find details about it in the contributing guidelines.

cxp@CXPfromR9000P:~/solana_learn/v2-core$ bulloak check test/core/integration/concrete/lockup/renounce-multiple/renounceMultiple.tree 
warn: an error occurred while parsing the solidity file
   --> test/core/integration/concrete/lockup/renounce-multiple/renounceMultiple.tree

warn: 1 check failed

I look the guide(https://github.com/alexfertel/bulloak?tab=readme-ov-file#installation). but it didn't show where the warn at the file.

@cxp-13
Copy link
Author

cxp-13 commented Oct 11, 2024

@cxp-13 before we review it, I'd appreciate if you can fix the failing CI. In case you ain't aware of the Bulloak, you can find details about it in the contributing guidelines.

Run forge test --match-path "test/{core,periphery}/fork/**/*.sol"
No files changed, compilation skipped

Ran 1 test for test/core/fork/assets/SHIB.t.sol:SHIB_LockupDynamic_Fork_Test
[FAIL: setup failed: vm.createSelectFork: Could not instantiate forked environment with fork url: ] setUp() (gas: 0)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 2.[14](https://github.com/sablier-labs/v2-core/actions/runs/11292070279/job/31410776543?pr=1061#step:6:15)ms (0.00ns CPU time)

Ran 1 test for test/core/fork/assets/USDC.t.sol:USDC_LockupLinear_Fork_Test
[FAIL: setup failed: vm.createSelectFork: Could not instantiate forked environment with fork url: ] setUp() (gas: 0)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 2.[15](https://github.com/sablier-labs/v2-core/actions/runs/11292070279/job/31410776543?pr=1061#step:6:16)ms (0.00ns CPU time)

My change don't involved the fork folder. but it's all test case are fail.

@smol-ninja
Copy link
Member

Don't worry about failing fork tests on CI. They don't work on PRs created by external contributors. We are still looking for a solution for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants