[WIP] Unify and extend rpc methods to bid and sequence #303

Merged
bvbfan merged 8 commits from rpc_hash_hardfork into master 2019-09-06 22:03:39 +02:00
bvbfan commented 2019-08-14 20:52:38 +02:00 (Migrated from github.com)

A bunch of updated rpc keys can lead to breaking changes in users, note it uses camelCase name notation, it can be changed as well. Notable changes:
nomalized_name -> normalizedName
txid -> txId
nHeight -> height
nValidAtHeight (valid at height) -> validAtHeight
nAmount -> amount
nEffectiveAmount (effective amount) -> effectiveAmount
total names -> totalNames
total claims -> totalClaims
total value -> totalValue
Most values in getclaimsfortx

A bunch of updated rpc keys can lead to breaking changes in users, note it uses camelCase name notation, it can be changed as well. Notable changes: nomalized_name -> normalizedName txid -> txId nHeight -> height nValidAtHeight (valid at height) -> validAtHeight nAmount -> amount nEffectiveAmount (effective amount) -> effectiveAmount total names -> totalNames total claims -> totalClaims total value -> totalValue Most values in getclaimsfortx
eukreign (Migrated from github.com) reviewed 2019-08-14 20:52:38 +02:00
BrannonKing (Migrated from github.com) reviewed 2019-08-14 20:52:38 +02:00
lbrynaut (Migrated from github.com) reviewed 2019-08-14 20:52:38 +02:00
tiger5226 (Migrated from github.com) reviewed 2019-08-21 03:10:40 +02:00
@ -0,0 +78,4 @@
" the latest active\n" \
" block will be used."
#define CLAIM_OUTPUT \
tiger5226 (Migrated from github.com) commented 2019-08-21 03:10:39 +02:00

These are less readable than before. I actually used these quite often.

These are less readable than before. I actually used these quite often.
tiger5226 (Migrated from github.com) reviewed 2019-08-21 03:11:45 +02:00
@ -133,0 +208,4 @@
CTxDestination address;
if (ExtractDestination(coin.out.scriptPubKey, address))
result.pushKV(T_ADDRESS, EncodeDestination(address));
tiger5226 (Migrated from github.com) commented 2019-08-21 03:11:44 +02:00

typo

typo
tiger5226 (Migrated from github.com) reviewed 2019-08-21 03:12:26 +02:00
@ -136,3 +280,4 @@
return count >= required && count <= required + optional;
}
void validateRequest(const JSONRPCRequest& request, int findex, uint8_t required, uint8_t optional)
tiger5226 (Migrated from github.com) commented 2019-08-21 03:12:26 +02:00

should we cast this?

should we cast this?
tiger5226 (Migrated from github.com) reviewed 2019-08-21 03:12:45 +02:00
@ -185,3 +305,3 @@
CBlockIndex *blockIndex = BlockHashIndex(ParseHashV(request.params[0], "blockhash (optional parameter 1)"));
CBlockIndex* blockIndex = BlockHashIndex(ParseHashV(request.params[0], T_BLOCKHASH " (optional parameter 1)"));
RollBackTo(blockIndex, coinsCache, trieCache);
}
tiger5226 (Migrated from github.com) commented 2019-08-21 03:12:45 +02:00

casting again, maybe the type needs to be updated?

casting again, maybe the type needs to be updated?
tiger5226 (Migrated from github.com) reviewed 2019-08-21 03:14:37 +02:00
@ -263,3 +345,3 @@
if (!request.params.empty()) {
CBlockIndex *blockIndex = BlockHashIndex(ParseHashV(request.params[0], "blockhash (optional parameter 1)"));
CBlockIndex* blockIndex = BlockHashIndex(ParseHashV(request.params[0], T_BLOCKHASH " (optional parameter 1)"));
RollBackTo(blockIndex, coinsCache, trieCache);
tiger5226 (Migrated from github.com) commented 2019-08-21 03:14:37 +02:00

nice catch!

nice catch!
tiger5226 (Migrated from github.com) reviewed 2019-08-21 03:20:51 +02:00
tiger5226 (Migrated from github.com) left a comment

Lots of changes here. I think it would be nice to just test the changes by validating the output between versions. It is largly a refactor of code, would have been nice to separate the refactor from the modifications required for bid and sequence. Is that possible?

Lots of changes here. I think it would be nice to just test the changes by validating the output between versions. It is largly a refactor of code, would have been nice to separate the refactor from the modifications required for bid and sequence. Is that possible?
bvbfan commented 2019-08-21 16:23:56 +02:00 (Migrated from github.com)

Changes are breaking since i unify outputs which makes changes to the fields.

Changes are breaking since i unify outputs which makes changes to the fields.
bvbfan (Migrated from github.com) reviewed 2019-08-21 16:25:25 +02:00
@ -0,0 +78,4 @@
" the latest active\n" \
" block will be used."
#define CLAIM_OUTPUT \
bvbfan (Migrated from github.com) commented 2019-08-21 16:25:25 +02:00

I think is more readable now :) You see it on one place and there is no need to verify it's same or not and it's correct on every place or not.

I think is more readable now :) You see it on one place and there is no need to verify it's same or not and it's correct on every place or not.
bvbfan (Migrated from github.com) reviewed 2019-08-21 16:28:18 +02:00
@ -136,3 +280,4 @@
return count >= required && count <= required + optional;
}
void validateRequest(const JSONRPCRequest& request, int findex, uint8_t required, uint8_t optional)
bvbfan (Migrated from github.com) commented 2019-08-21 16:28:18 +02:00

UniValue does not provide an uint32 constructor which can lead to ambiguous.

UniValue does not provide an uint32 constructor which can lead to ambiguous.
tiger5226 (Migrated from github.com) reviewed 2019-08-23 03:50:04 +02:00
@ -0,0 +78,4 @@
" the latest active\n" \
" block will be used."
#define CLAIM_OUTPUT \
tiger5226 (Migrated from github.com) commented 2019-08-23 03:50:03 +02:00

I was also looking at it through the PR lens. Will look at it in the code base too.

I was also looking at it through the PR lens. Will look at it in the code base too.
bvbfan (Migrated from github.com) reviewed 2019-08-23 13:23:17 +02:00
@ -0,0 +78,4 @@
" the latest active\n" \
" block will be used."
#define CLAIM_OUTPUT \
bvbfan (Migrated from github.com) commented 2019-08-23 13:23:17 +02:00

Overall code base is way better to me, logic and help message are separate which makes logic to be easy follow, OTOH help message are minimize.

Overall code base is way better to me, logic and help message are separate which makes logic to be easy follow, OTOH help message are minimize.
BrannonKing commented 2019-09-05 23:50:58 +02:00 (Migrated from github.com)

The cli help (that shows all the available methods) isn't showing the parameters for the methods in rpc/claimtrie.cpp. It's like the argNames are ignored in that file.

The cli help (that shows all the available methods) isn't showing the parameters for the methods in rpc/claimtrie.cpp. It's like the argNames are ignored in that file.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbrycrd#303
No description provided.