Finish Rosetta Endpoints #1050

LindaOrtega · 2020-05-14T15:59:23Z

Future Work:

Add gas price and gas limit in metadata for /mempool/transaction and transaction in general.
When creating Operations figure out how to efficiently add "related operations" (an optional Rosetta field in the Operation type). Already including TxId and operation id is generated based on the order the logs appeared in.
Investigate how Rosetta validator tool handles continuations and cross-chain transactions. Especially if cross-chain wrapped inside of another function.
Add Rosetta endpoint tests #1073
Enable Rosetta config #1072
Add Show instances to Rosetta types for better debugging: Add Show instances rosetta#9

…web-node into rosetta-block-endpoints

…tta-block-endpoints

…web-node into rosetta-block-endpoints

emilypi

First past done, but I need to stare at the algorithm for txlogs some more before approval

src/Chainweb/Rosetta/RestAPI.hs

emilypi · 2020-05-20T18:14:29Z

src/Chainweb/Rosetta/RestAPI.hs

+  RosettaError 7 "Historical account balance lookup is not supported." False
+rosettaError RosettaPactExceptionThrown =
+  RosettaError 7 "A pact exception was thrown" False
+rosettaError RosettaPactErrorThrown = RosettaError 8 "Transaction failed with a pact error" False


Is it spec that Rosetta be mixed up with Pact like this? Why would Rosetta care if a tx failed? That's still a valid tx to the blockchain.

This is only thrown when looking up account balance fails with a pact error, which (assuming I constructed the local command correctly) should only happen if the account doesn't exist. I changed it to something more user friendly: RosettaCouldNotRetrieveBalance No longer applies. When account not present, just return a balance of 0.0.

@emilypi
The only time a specific "Pact" rosetta error thrown is via RosettaPactExceptionThrown. This is thrown by getTxLogs and getHistoricalLookupBalance, when the internal pact API throws an error. Not sure if this should be encapsulated into another less specific Rosetta Error.

src/Chainweb/Rosetta/RestAPI/Server.hs

…eb-node into rosetta-block-endpoints

…nsactionHash

…tta-block-endpoints

LindaOrtega · 2020-06-09T22:24:49Z

src/Chainweb/Rosetta/Util.hs

+    -> T.Text
+    -> ExceptT RosettaFailure Handler Decimal
+getHistoricalLookupBalance cr bh k = do
+  someHist <- liftIO $ (_pactHistoricalLookup cr) bh d key


Could this be subject to sql injection? Is it okay to use the bare text submitted by the user when its going to be used to form sql statements (sql statement used: https://github.com/kadena-io/chainweb-node/pull/1050/files#diff-1cee5293be7515e25b6f00d33b117ac5R333-R339)?

Yes, it's a concern. @emilypi is there Haskell code (as opposed to the pact code in coin) for validating account IDs somewhere?

Also, in the future you should consider possibly making _pactHistoricalLookup return the last two transactions (since your SQL is already a LIMIT 1 affair). Why? Because we can do deltas with that! 😎

I don't believe we have haskell code for this. We only validate decimal inputs iirc, but we can definitely write that or reuse this code if we have access to a pact db handel:

chainweb-node/src/Chainweb/Pact/PactService.hs

Line 876 in affe0b4

readCoinAccount

@slpopejoy regarding returning the last two transactions, I've made an issue for it here: #1078

@slpopejoy and @emilypi regarding the potential sql injection via account address: I've added the following code to check that the account address is a valid Pact Name (since I believe row keys need to be valid Name correct?)
https://github.com/kadena-io/chainweb-node/pull/1050/files#diff-04e165a290c33e67270413e0ae8c3d85R515-R519

Actually, I need to reconsider this approach. It throws an errors with "address": "POTENT AF MINER" on mainnet. This seems to be to strict

sirlensalot

Wow what a huge effort! The two things I wish were less complex but seems unavoidable at this point:

The fork-checking for the PartialBlockId stuff. I can't really see around it, but Checkpointer is already at the "current fork" so if somehow we could just send that straight to Checkpointer you wouldn't have to bother.
The heuristics for assembling the txlog triples. No idea what would be a better way there ...
Finally the indirection I noted, you don't have to change it but I find the code in Util a little hard to follow. Fine to leave it for now in interests of delivery.

Requesting changes mainly for the genesis block check stuff in checkpointer.

src/Chainweb/Pact/Backend/RelationalCheckpointer.hs

sirlensalot · 2020-06-10T02:38:56Z

src/Chainweb/Rosetta/Util.hs

+    -> T.Text
+    -> ExceptT RosettaFailure Handler Decimal
+getHistoricalLookupBalance cr bh k = do
+  someHist <- liftIO $ (_pactHistoricalLookup cr) bh d key


Yes, it's a concern. @emilypi is there Haskell code (as opposed to the pact code in coin) for validating account IDs somewhere?

src/Chainweb/Rosetta/Util.hs

sirlensalot · 2020-06-10T02:59:19Z

src/Chainweb/Rosetta/Util.hs

+       nonGenesisTransaction'
+       logs _crReqKey _crTxId rosettaTransaction initial rest target
+
+nonGenesisTransaction'


Not seeing why there's the function indirection here (getXXX) arguments since this is only used once, is that for tests?

Also same comment as above regarding polymorphism and roles of a and b, it makes it hard to understand what is being accomplished here.

Got rid of the getXXX indirection. And yes part of the indirection was there for the tests because debugging is easier with Either String rosettaTx. But I added the conversion of the Either String rosettaTx to the matchLogs function.

@slpopejoy @emilypi I tried to simplify the matching logic and cut down on the polymorphism. See this commit for exactly what changed: ee355f5

src/Chainweb/Rosetta/Util.hs

sirlensalot · 2020-06-10T03:16:41Z

src/Chainweb/Rosetta/Util.hs

+    -> T.Text
+    -> ExceptT RosettaFailure Handler Decimal
+getHistoricalLookupBalance cr bh k = do
+  someHist <- liftIO $ (_pactHistoricalLookup cr) bh d key


Also, in the future you should consider possibly making _pactHistoricalLookup return the last two transactions (since your SQL is already a LIMIT 1 affair). Why? Because we can do deltas with that! 😎

src/Chainweb/Pact/Backend/InMemoryCheckpointer.hs

src/Chainweb/Pact/Backend/RelationalCheckpointer.hs

emilypi · 2020-06-10T17:41:44Z

src/Chainweb/Rosetta/Util.hs

+    -> T.Text
+    -> ExceptT RosettaFailure Handler Decimal
+getHistoricalLookupBalance cr bh k = do
+  someHist <- liftIO $ (_pactHistoricalLookup cr) bh d key


I don't believe we have haskell code for this. We only validate decimal inputs iirc, but we can definitely write that or reuse this code if we have access to a pact db handel:

chainweb-node/src/Chainweb/Pact/PactService.hs

Line 876 in affe0b4

readCoinAccount

…-node into rosetta-block-endpoints

Co-authored-by: Emily Pillmore <emily@kadena.io>

…tta-block-endpoints

…eb-node into rosetta-block-endpoints

src/Chainweb/Pact/Backend/RelationalCheckpointer.hs

src/Chainweb/Rosetta/Internal.hs

src/Chainweb/Rosetta/Util.hs

larskuhtz

I didn't do a full review, but checked that there are not adverse effects for other chainweb components. 👍

LindaOrtega and others added 20 commits May 12, 2020 19:00

adds account balance handler

6040ebe

slight cleanup -- wip block info in local

85f6152

Made account balance command use term name

9ee7a94

WIP start setting up types for txlog lookup

722b31a

more wip

4c563f2

Merge branch 'rosetta-account-endpoint' of github.com:kadena-io/chain…

dbd0deb

…web-node into rosetta-block-endpoints

WIP - adds logic for creating rosetta tx given tx logs and req key

5963ee9

WIP - code cleanup, finalized tx log parsing logic

7ec9f7b

Merge branch 'master' of github.com:kadena-io/chainweb-node into rose…

ade7063

…tta-block-endpoints

Adds logic for finding block header in curr fork by height and or hash

690d1fe

Compiles -- adds logic for getting payload outputs

65adb1c

tested locally, found couple of bugs

c28e3aa

fixes: genesis parsing issues, missing block coinbase tx

b5fdde4

Merge branch 'master' into rosetta-account-endpoint

a3d9d4d

Merge branch 'rosetta-account-endpoint' of github.com:kadena-io/chain…

cfdd3ce

…web-node into rosetta-block-endpoints

fixes tx matching, improves performance

5debe86

add mempool endpoint impl

5576553

Merge branch 'master' into rosetta-mempool-endpoint

f296b3b

final /block cleanup

0472534

Merge branch 'master' into rosetta-block-endpoints

1a3cf4a

LindaOrtega marked this pull request as ready for review May 20, 2020 18:08

emilypi reviewed May 20, 2020

View reviewed changes

LindaOrtega added 4 commits May 20, 2020 18:43

Some refactoring/documentation in prep for /block/transaction

578ab92

Merge branch 'rosetta-block-endpoints' of github.com:kadena-io/chainw…

03488b0

…eb-node into rosetta-block-endpoints

logic for /block/transaction compiling

8c690d9

finish up /block/transaction and /network/options

dd1819a

LindaOrtega changed the title ~~Add /block endpoints~~ Finish Rosetta Endpoints May 21, 2020

LindaOrtega and others added 3 commits May 21, 2020 17:13

WIP - begin refactoring

a0bda21

force highwater mark

895079b

use mvar instead

7ce3e93

LindaOrtega added 3 commits June 3, 2020 13:25

Adds historical account lookup

7f0796d

Fix rosetta validator issues -- remediations, error codes

81cdfd6

Fix mempool error -- use hastextrepresentation for transactionId->Tra…

c28f21e

…nsactionHash

LindaOrtega mentioned this pull request Jun 9, 2020

Adds /account/balance endpoint #1047

Closed

2 tasks

LindaOrtega added 2 commits June 9, 2020 18:04

Merge branch 'master' of github.com:kadena-io/chainweb-node into rose…

4201c99

…tta-block-endpoints

undo changes no longer needed

e479699

LindaOrtega commented Jun 9, 2020

View reviewed changes

sirlensalot requested changes Jun 10, 2020

View reviewed changes

emilypi reviewed Jun 10, 2020

View reviewed changes

emilypi and others added 8 commits June 11, 2020 15:42

switch rosetta config back on

5d0c019

first pass at dealing with edge case where coinbase fails

82935e8

Merge branch 'expose-rosetta-config' of github.com:kadena-io/chainweb…

1fcb891

…-node into rosetta-block-endpoints

Review edits -- refactor matching logic, etc

ee355f5

Review edits -- factor out tx matching and block helpers into own module

75e7bb7

Update src/Chainweb/Pact/Backend/InMemoryCheckpointer.hs

aabc8a5

Co-authored-by: Emily Pillmore <emily@kadena.io>

Merge branch 'master' of github.com:kadena-io/chainweb-node into rose…

21e7b72

…tta-block-endpoints

Merge branch 'rosetta-block-endpoints' of github.com:kadena-io/chainw…

3c40718

…eb-node into rosetta-block-endpoints

LindaOrtega commented Jun 15, 2020

View reviewed changes

src/Chainweb/Pact/Backend/RelationalCheckpointer.hs Show resolved Hide resolved

LindaOrtega commented Jun 15, 2020

View reviewed changes

src/Chainweb/Rosetta/Internal.hs Show resolved Hide resolved

LindaOrtega commented Jun 15, 2020

View reviewed changes

src/Chainweb/Rosetta/Util.hs Show resolved Hide resolved

Cleaned up some unused rosetta error types

560d95f

sirlensalot approved these changes Jun 15, 2020

View reviewed changes

LindaOrtega requested a review from emilypi June 15, 2020 18:49

larskuhtz approved these changes Jun 15, 2020

View reviewed changes

LindaOrtega and others added 3 commits June 15, 2020 18:27

Disable checking account address is valid Name

8634918

Finish undoing too strict sanitization of account key

808bc69

Merge branch 'master' into rosetta-block-endpoints

c9f27f9

emilypi approved these changes Jun 18, 2020

View reviewed changes

emilypi merged commit d75df80 into master Jun 18, 2020

emilypi deleted the rosetta-block-endpoints branch June 18, 2020 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finish Rosetta Endpoints #1050

Finish Rosetta Endpoints #1050

LindaOrtega commented May 14, 2020 •

edited by emilypi

Loading

emilypi left a comment

emilypi May 20, 2020

LindaOrtega May 27, 2020 •

edited

Loading

LindaOrtega Jun 9, 2020 •

edited

Loading

sirlensalot Jun 10, 2020

sirlensalot Jun 10, 2020

emilypi Jun 10, 2020

LindaOrtega Jun 15, 2020

LindaOrtega Jun 15, 2020

sirlensalot left a comment

sirlensalot Jun 10, 2020

sirlensalot Jun 10, 2020

sirlensalot Jun 10, 2020

LindaOrtega Jun 15, 2020

sirlensalot Jun 10, 2020

emilypi Jun 10, 2020

larskuhtz left a comment

Finish Rosetta Endpoints #1050

Finish Rosetta Endpoints #1050

Conversation

LindaOrtega commented May 14, 2020 • edited by emilypi Loading

emilypi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LindaOrtega May 27, 2020 • edited Loading

Choose a reason for hiding this comment

LindaOrtega Jun 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sirlensalot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larskuhtz left a comment

Choose a reason for hiding this comment

LindaOrtega commented May 14, 2020 •

edited by emilypi

Loading

LindaOrtega May 27, 2020 •

edited

Loading

LindaOrtega Jun 9, 2020 •

edited

Loading