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

SON object and operations #144

Closed
wants to merge 17 commits into from

Conversation

oxarbitrage
Copy link

Replaces #48

I toke the initial work from @pixelplex , made some naming changes, added the update operation, added hardfork guards and other small changes.

The next step should be to review and port the code from b3786b3 into here in one side, this will allow to vote for this objects.

In the other side we can follow with creating the needed cli_wallet commands described in internal document. In order to do this we are going to need the cli_wallet tests from develop.

@obucina
Copy link

obucina commented Sep 24, 2019

Building error, SONs-base + 144

peerplays/peerplays/libraries/chain/proposal_evaluator.cpp:144:32: error: ‘HARDFORK_SON_TIME’ was not declared in this scope
FC_ASSERT( block_time >= HARDFORK_SON_TIME, "son_create_operation not allowed yet!" );

@oxarbitrage
Copy link
Author

my apologies, a file was missing.

@obucina
Copy link

obucina commented Sep 27, 2019

@oxarbitrage

Regarding vote_id_type::son

database_api.cpp
vector database_api_impl::lookup_vote_ids( const vector<vote_id_type>& votes )const

There is a switch inside this function, and it does not have vote_id_type::son as one of the cases.

Should it be there?

@obucina obucina self-requested a review September 27, 2019 17:33
@obucina
Copy link

obucina commented Sep 27, 2019

One more question about voting... With this PR, is SON voting functional, and how we can verify this? Can you provide a curl command or anything else?

@oxarbitrage
Copy link
Author

This pull request is for this document: https://peerplays.atlassian.net/wiki/spaces/PIX/pages/333971489/SON+Objects+and+Operators
So, regarding to your question, we cant say voting for son is functional with this pull request.

The voting is described here as far as i know: https://peerplays.atlassian.net/wiki/spaces/PIX/pages/342261826/SON+Voting+LLD

And there is an attempt of implementation where some parts will be maybe reusable at: #49

In regards to the lookup_vote_ids, i added code for it, nice find.

@oxarbitrage
Copy link
Author

IMHO, the document about voting is confusing: https://peerplays.atlassian.net/wiki/spaces/PIX/pages/342261826/SON+Voting+LLD

It introduce new fields to the son object and new fields and functionality(users must have a vesting balance to register)to the create operation in compassion with the former document, it introduces the delete operation again, it mentions sidechain diagrams that are unrelated.
The modifications into the operations and objects should be moved into https://peerplays.atlassian.net/wiki/spaces/PIX/pages/333971489/SON+Objects+and+Operators where they belong. The unrelated sidechain things should be removed and placed in somewhere else. The voting document should specify only how son members can became active or inactive by voting(very similar to witness functionality) and nothing more.

@bobinson
Copy link

@satyakoneru : please look into the comments on LLDs here.

tests/tests/son_operations_tests.cpp Outdated Show resolved Hide resolved
@bobinson
Copy link

bobinson commented Sep 29, 2019 via email

@obucina
Copy link

obucina commented Oct 1, 2019

Quick question... Is this ready for a merge? Does it have all thats planned?

@oxarbitrage
Copy link
Author

I had been out the last week but i think there was 1 thing missing here that was the vesting balance you need to have locked to became a son member. Needs to be tied to the create operation. Besides that the main things are all done. I will review the documents in case anything was changed or added and will add that functionality to merge if you agree. Voting should be done in a new pull request.

@obucina
Copy link

obucina commented Oct 7, 2019

I agree. Please finish vesting balance as soon as possible. Thanks

@oxarbitrage
Copy link
Author

Most of the code in here was already merged to feature/SONs-base at #160 and the rest has now its own pr at #183

Closing this one as it is not needed anymore.

@oxarbitrage oxarbitrage deleted the son_objects_operations branch October 16, 2019 01:09
@pbsa-github pbsa-github restored the son_objects_operations branch May 19, 2020 14:15
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.

4 participants