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.

Sign up for GitHub

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

Jump to bottom

feat(ICP-Rosetta): FI-1540: add disburse of neuron functionality #2182

Merged
merged 16 commits into from
Oct 24, 2024
Merged

feat(ICP-Rosetta): FI-1540: add disburse of neuron functionality #2182

merged 16 commits into from
Oct 24, 2024

Conversation

NikolasHai
Copy link
Contributor

This MR proposes the following changes:

  1. Add the functionality of disbursing a neuron to the ICP Rosetta client

@github-actions github-actions bot added the feat label Oct 22, 2024
@NikolasHai NikolasHai marked this pull request as ready for review October 24, 2024 11:17
@NikolasHai NikolasHai requested a review from a team as a code owner October 24, 2024 11:17
mbjorkqvist
mbjorkqvist approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@mbjorkqvist mbjorkqvist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @NikolasHai !

IIUC, these are all new tests, and not refactored old system tests - if so, would it make sense to ask the NNS team to have a quick look as well to double-check that the neuron management is done as they would expect (I'm not sure I would consider myself an expert on this subject!)?

rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_management.rs Show resolved Hide resolved
rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_management.rs Outdated Show resolved Hide resolved
rs/rosetta-api/icp/client/src/lib.rs Outdated Show resolved Hide resolved
@NikolasHai NikolasHai requested a review from a team as a code owner October 24, 2024 11:42
@github-actions github-actions bot added the @idx label Oct 24, 2024
@NikolasHai @mbjorkqvist
Update rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_manage…
d6c3c45 
…ment.rs

Co-authored-by: Mathias Björkqvist <mathias.bjorkqvist@dfinity.org>
@NikolasHai
Copy link
Contributor Author

LGTM, thanks @NikolasHai !

IIUC, these are all new tests, and not refactored old system tests - if so, would it make sense to ask the NNS team to have a quick look as well to double-check that the neuron management is done as they would expect (I'm not sure I would consider myself an expert on this subject!)?

These tests should replace the old system tests and effectively test the same functionality.

@NikolasHai NikolasHai added this pull request to the merge queue Oct 24, 2024
Merged via the queue into master with commit c705212 Oct 24, 2024
25 checks passed
@NikolasHai NikolasHai deleted the FI-1540-add-clien-function-for-disbursing-a-neuron branch October 24, 2024 15:19
nmattia pushed a commit that referenced this pull request Oct 25, 2024
@NikolasHai @mbjorkqvist @nmattia
feat(ICP-Rosetta): FI-1540: add disburse of neuron functionality (#2182)
1845348 
This MR proposes the following changes:

1. Add the functionality of disbursing a neuron to the ICP Rosetta
client

---------

Co-authored-by: Mathias Björkqvist <mathias.bjorkqvist@dfinity.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basvandijk basvandijk basvandijk approved these changes

@mbjorkqvist mbjorkqvist mbjorkqvist approved these changes

Assignees
No one assigned
Labels
feat @finint @idx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NikolasHai @basvandijk @mbjorkqvist