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

Adds staking ability to transfer_split RPC, Don't provide RTA-related… #304

Merged
merged 4 commits into from
Jun 20, 2019
Merged

Adds staking ability to transfer_split RPC, Don't provide RTA-related… #304

merged 4 commits into from
Jun 20, 2019

Conversation

Fez29
Copy link
Contributor

@Fez29 Fez29 commented May 30, 2019

Don't provide RTA-related RPC endpoints in restricted mode

graft-community@c19d6dc

Adds staking ability to transfer_split RPC

graft-community@374d76f

Added parameters are:

stake_transfer - must be true to do a staking transfer
supernode_public_id - the SN pubkey
supernode_signature - a SN signature value
allow_low_stake - set to true to allow staking less than a T1 (for
                  example, to submit an "upgrade" stake amount).

The transfer must also be sent to a single address (which has to match
the one configured in the SN for the signature to be valid).

mbg033
mbg033 requested changes May 31, 2019
View reviewed changes
src/wallet/wallet_rpc_server_commands_defs.h Outdated
std::string supernode_public_id;
std::string supernode_signature;
bool allow_low_stake;

Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear why these fields has been added to COMMAND_RPC_CREATE_ADDRESS command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, apologies patch went wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

@Fez29, now I'm getting build error while building your branch:

[ 81%] Building CXX object tests/fuzz/CMakeFiles/cold-outputs_fuzz_tests.dir/cold-outputs.cpp.o
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp: In member function ‘bool tools::wallet_rpc_server::on_transfer(const tools::wallet_rpc::COMMAND_RPC_TRANSFER::request&, tools::wallet_rpc::COMMAND_RPC_TRANSFER::response&, epee::json_rpc::error&)’:
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:838:13: error: ‘const struct tools::wallet_rpc::COMMAND_RPC_TRANSFER::request’ has no member named ‘stake_transfer’
     if (req.stake_transfer) {
             ^~~~~~~~~~~~~~
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:840:47: error: ‘const struct tools::wallet_rpc::COMMAND_RPC_TRANSFER::request’ has no member named ‘supernode_signature’
       if (!epee::string_tools::hex_to_pod(req.supernode_signature, supernode_signature))
                                               ^~~~~~~~~~~~~~~~~~~
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:848:47: error: ‘const struct tools::wallet_rpc::COMMAND_RPC_TRANSFER::request’ has no member named ‘supernode_public_id’
       if (!epee::string_tools::hex_to_pod(req.supernode_public_id, W) || !check_key(W))
                                               ^~~~~~~~~~~~~~~~~~~
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:855:134: error: cannot convert ‘bool’ to ‘cryptonote::network_type’ for argument ‘1’ to ‘std::__cxx11::string cryptonote::get_account_address_as_str(cryptonote::network_type, bool, const cryptonote::account_public_address&)’
       std::string supernode_public_address_str = cryptonote::get_account_address_as_str(m_wallet->testnet(), supernode_public_address);
                                                                                                                                      ^
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:856:67: error: ‘const struct tools::wallet_rpc::COMMAND_RPC_TRANSFER::request’ has no member named ‘supernode_public_id’
       std::string data = supernode_public_address_str + ":" + req.supernode_public_id;
                                                                   ^~~~~~~~~~~~~~~~~~~
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:890:62: error: ‘const struct tools::wallet_rpc::COMMAND_RPC_TRANSFER::request’ has no member named ‘allow_low_stake’
       if (amount < config::graft::TIER1_STAKE_AMOUNT && !req.allow_low_stake)
                                                              ^~~~~~~~~~~~~~~
/home/mbg033/dev/graft-project/GraftNetwork/src/wallet/wallet_rpc_server.cpp:897:57: error: ‘const struct tools::wallet_rpc::COMMAND_RPC_TRANSFER::request’ has no member named ‘supernode_public_id’
       if (!add_graft_stake_tx_extra_to_extra(extra, req.supernode_public_id, supernode_public_address, supernode_signature))
                                                         ^~~~~~~~~~~~~~~~~~~
src/wallet/CMakeFiles/wallet_rpc_server.dir/build.make:62: recipe for target 'src/wallet/CMakeFiles/wallet_rpc_server.dir/wallet_rpc_server.cpp.o' failed

Copy link
Contributor

Choose a reason for hiding this comment

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

@Fez29, still having build errors.
Also, not clear what was the intent - transfer_split mentioned in PR title, stake transaction related fields were added to COMMAND_RPC_TRANSFER_SPLIT but handled in on_transfer - guess 'stake' related fields should be added into COMMAND_RPC_TRANSFER as well and handled same way in both on_transfer and on_transfer_split ?

mbg033
mbg033 requested changes Jun 4, 2019
View reviewed changes
src/wallet/wallet_rpc_server_commands_defs.h Outdated
std::string supernode_public_id;
std::string supernode_signature;
bool allow_low_stake;

Copy link
Contributor

Choose a reason for hiding this comment

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

@Fez29, still having build errors.
Also, not clear what was the intent - transfer_split mentioned in PR title, stake transaction related fields were added to COMMAND_RPC_TRANSFER_SPLIT but handled in on_transfer - guess 'stake' related fields should be added into COMMAND_RPC_TRANSFER as well and handled same way in both on_transfer and on_transfer_split ?

mbg033
mbg033 requested changes Jun 6, 2019
View reviewed changes
src/wallet/wallet_rpc_server.cpp
return false;
}

if (dsts.size() != 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

transfer_split expects more than one destinations on input, but in case "stake_transfer" there should be one destination. It looks confusing, please move this functionality to transfer

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please squash your commits related to stake_transfer into one commit

@avastar avastar mentioned this pull request Jun 16, 2019
Closed
@avastar
Copy link
Contributor

avastar commented Jun 16, 2019 •
edited
Loading

i need to submit a pull request using a branch.. sorry. will resubmit soon

@avastar avastar mentioned this pull request Jun 16, 2019
Merged
@mbg033 mbg033 merged commit ff4ed03 into graft-project:master Jun 20, 2019
mbg033 added a commit that referenced this pull request Jun 20, 2019
@mbg033
Merge pull request #320 from avastar/avastar-rpc-stake-transfer
9c53830 
completes PR #304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbg033 mbg033 mbg033 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Fez29 @avastar @mbg033