Upgrade to latest rosetta sdk #1975

xin-hedera · 2021-05-12T20:23:00Z

Detailed description:

Upgrade to rosetta-sdk-go v0.6.10 with support of rosetta-specifications@v1.4.10
Add construction validation configuration file and test scenario DSL file
Build transaction balance-changing operations from both non-fee transfers and crypto transfers
Fix /network/options returning all transaction results as successful
Fix /construction/preprocess not returning required public keys
Fix /construction/parse returning account pubic key instead of account ID
Fix /construction/payloads not returning signature type
Fix /block returning transactions not after genesis balance timestamp
Fix run-validation.sh always install rosetta-cli
Upgrade to gorm v1.21.10 (v2) with db connection pool params to fix db connection failure
Use First() to get ErrRecordNotFound as it's a gorm v2 breaking change
Enable persisting NonFeeTransfers for importer in the all-in-one Dockerfile
Add script to retrieve genesis account balances
Add construction API validation job to rosetta-api workflow

Which issue(s) this PR fixes:
Fixes #1918

Special notes for your reviewer:
The validate_construction job needs new github secrets to work, before that, it'll fail. The new secrets are:

base64 encoded application.yml file with testnet cloud storage credentials
base64 encoded json file with two prefunded testnet accounts (account id, private key, and etc)

Checklist

Documentation added
Tests updated

Signed-off-by: Xin Li <xin.li@hedera.com>

codecov · 2021-05-12T20:25:48Z

Codecov Report

Merging #1975 (4e88b9c) into master (126751e) will decrease coverage by 1.75%.
The diff coverage is 82.88%.

@@             Coverage Diff              @@
##             master    #1975      +/-   ##
============================================
- Coverage     87.05%   85.30%   -1.76%     
- Complexity     1744     1822      +78     
============================================
  Files           315      277      -38     
  Lines          7731     6300    -1431     
  Branches        740      470     -270     
============================================
- Hits           6730     5374    -1356     
+ Misses          772      682      -90     
- Partials        229      244      +15

Impacted Files	Coverage Δ	Complexity Δ
...ra/mirror/monitor/config/MonitorConfiguration.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../hedera/mirror/monitor/publish/PublishMetrics.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...a/mirror/monitor/publish/TransactionPublisher.java	`1.06% <0.00%> (-0.08%)`	`1.00 <0.00> (ø)`
...a/mirror/monitor/subscribe/AbstractSubscriber.java	`77.14% <ø> (-20.00%)`	`3.00 <0.00> (-1.00)`
.../mirror/monitor/subscribe/rest/RestSubscriber.java	`100.00% <ø> (ø)`	`22.00 <0.00> (?)`
...nitor/subscribe/rest/RestSubscriberProperties.java	`83.33% <ø> (ø)`	`4.00 <0.00> (?)`
hedera-mirror-rosetta/cmd/db.go	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
hedera-mirror-rosetta/tools/maphelper/maphelper.go	`100.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...ra/mirror/monitor/subscribe/SubscribeResponse.java	`66.66% <66.66%> (ø)`	`4.00 <4.00> (?)`
hedera-mirror-rosetta/cmd/config.go	`66.66% <67.74%> (+66.66%)`	`0.00 <0.00> (ø)`
... and 88 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 847069e...4e88b9c. Read the comment docs.

Signed-off-by: Xin Li <xin.li@hedera.com>

hedera-mirror-rosetta/scripts/validation/run-validation.sh

.github/workflows/rosetta-api.yml

hedera-mirror-rosetta/app/persistence/common/common.go

hedera-mirror-rosetta/app/services/construction/construction_service.go

Nana-EC

Looking good on 1st pass, will circle back

Nana-EC

Some thoughts

Nana-EC · 2021-05-14T21:18:35Z

.github/workflows/rosetta-api.yml

+      - name: Load Docker image
+        run: |
+          docker load --input /tmp/test-image.tar
+          docker image ls -a


This also

Suggested change

docker image ls -a

this step is removed

Nana-EC · 2021-05-14T21:26:18Z

hedera-mirror-rosetta/app/domain/types/operation.go

@@ -41,7 +41,7 @@ func (t *Operation) ToRosetta() *rTypes.Operation {
 		},
 		RelatedOperations: []*rTypes.OperationIdentifier{},
 		Type:              t.Type,
-		Status:            t.Status,
+		Status:            &t.Status,


q: why the pointer only for Status?

Is the status changed after transform?

struct defined by rosetta-sdk-go. it's a breaking change, somewhere in their changelog should have it highlighted as well as the motivation :)

Gotcha, yeah a minor highlight might be useful, thanks

hedera-mirror-rosetta/app/persistence/common/common.go

hedera-mirror-rosetta/app/persistence/transaction/transaction.go

hedera-mirror-rosetta/app/services/account/account_service.go

Signed-off-by: Xin Li <xin.li@hedera.com>

- fix run-validation always install rosetta-cli Signed-off-by: Xin Li <xin.li@hedera.com>

- add db pool config Signed-off-by: Xin Li <xin.li@hedera.com>

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera · 2021-05-18T16:37:56Z

hedera-mirror-rosetta/test/mocks/mocks.go

+var sqlNamedParamRe = regexp.MustCompile(`(@[^ ,)"'\n]*)`)
+
+// replaces named parameter to indexed format $1, $2, ...
+var queryMatcher sqlmock.QueryMatcher = sqlmock.QueryMatcherFunc(func(expectedSQL, actualSQL string) error {
+	namedParams := sqlNamedParamRe.FindAllString(expectedSQL, -1)
+
+	index := 1
+	namedIndexes := make(map[string]string)
+	for _, name := range namedParams {
+		if _, ok := namedIndexes[name]; !ok {
+			namedIndexes[name] = fmt.Sprintf("$%d", index)
+			index++
+		}
+	}
+
+	for name, indexStr := range namedIndexes {
+		expectedSQL = strings.ReplaceAll(expectedSQL, name, indexStr)
+	}
+
+	return sqlmock.QueryMatcherRegexp.Match(regexp.QuoteMeta(expectedSQL), actualSQL)
+})
+


gorm v2 no longer supports postgresql index style param placeholder as the input SQL query. All the raw queries are converted to gorm v2's named param style (starts with @).

gorm v2 translates the SQL query's named param to index style placeholder (since gorm is configured with postgresql dialect) before it's captured by the mock.

the queryMatcher does the same translation so in our tests we can pass the SQL query and its param values match.

we really should test against real db instead.

xin-hedera · 2021-05-18T16:41:49Z

hedera-mirror-rosetta/scripts/validation/run-validation.sh


-if (./rosetta-cli 2>&1 | grep -q 'CLI for the Rosetta API'); then
+if (./rosetta-cli 2>&1 | grep 'CLI for the Rosetta API' > /dev/null); then


grep -q with the pipe and set -o pipefail will cause error code 141 since grep -q exits once a match is found, without consuming all the data from the pipe, thus rosetta-cli is always being installed.

xin-hedera · 2021-05-18T16:44:17Z

hedera-mirror-rosetta/scripts/validation/get-genesis-balance.sh

+with genesis_balance as (
+  select account_id, balance
+  from ((select min(consensus_timestamp) as timestamp from account_balance_file)) as genesis
+  join crypto_transfer ct
+    on ct.consensus_timestamp > genesis.timestamp
+  join account_balance ab
+    on ab.account_id = ct.entity_id and ab.consensus_timestamp = genesis.timestamp
+  where balance <> 0
+  group by account_id,balance
+  order by min(ct.consensus_timestamp)
+  limit 20
+)
+select json_agg(json_build_object('id', account_id::text, 'value', balance::text))
+from genesis_balance


the query selects up to 20 accounts' genesis balance. the accounts are ordered by their first crypto transfer after the genesis balance timestamp

needs the CTE otherwise the json_agg will repeat every account & balance pair for the number of occurencies as if there is no group by

xin-hedera · 2021-05-18T16:45:25Z

hedera-mirror-rosetta/build/Dockerfile

+# Enable persisting non-fee transfers to reconstruct the original transfer list
+ENV HEDERA_MIRROR_IMPORTER_PARSER_RECORD_ENTITY_PERSIST_NONFEETRANSFERS=true
+# Disable Redis because gRPC process is not run within this execution
+ENV HEDERA_MIRROR_IMPORTER_PARSER_RECORD_ENTITY_REDIS_ENABLED=false


this is moved from run_supervisord.sh

xin-hedera · 2021-05-18T16:50:33Z

hedera-mirror-rosetta/app/persistence/block/block.go

+	// selectGenesis - Selects the first block whose consensus_end is after the genesis account balance
+	// timestamp. Return the record file with adjusted consensus start
+	selectGenesis string = `SELECT
+                              consensus_end,
+                              hash,
+                              index,
+                              prev_hash,
+                              CASE
+                                WHEN genesis.min >= rf.consensus_start THEN genesis.min + 1
+                                ELSE rf.consensus_start
+                              END AS consensus_start
+                            FROM record_file AS rf
+                            JOIN (SELECT MIN(consensus_timestamp) FROM account_balance_file) AS genesis
+                              ON consensus_end > genesis.min
+                            ORDER BY consensus_end
+                            LIMIT 1`


The query finds the genesis record file and adjusts its consensus start to make sure that consensus start > first ingested account balance file consensus timestamp

xin-hedera · 2021-05-18T16:52:20Z

hedera-mirror-rosetta/app/persistence/block/block.go

+	index += br.genesisRecordFileIndex
+	if index == br.genesisRecordFileIndex {
+		rf = br.genesisRecordFile
+	} else if err := br.dbClient.Raw(selectRecordFileByIndex, sql.Named("index", index)).First(rf).Error; err != nil {


gorm v2's breaking change: only First, Last, and Take return ErrRecordNotFound

xin-hedera · 2021-05-18T16:55:46Z

.github/workflows/rosetta-api.yml

+          echo "Installing Rosetta CLI"
+          curl -sSfL https://raw.githubusercontent.com/coinbase/rosetta-cli/master/scripts/install.sh | sh -s -- -b .


install rosetta-cli before running the scripts otherwise the two processes may run into race condition

Signed-off-by: Xin Li <xin.li@hedera.com>

- refactor errors - increase validation timeout to 20 mins Signed-off-by: Xin Li <xin.li@hedera.com>

Nana-EC

Looks good, one nit suggestion

Nana-EC · 2021-05-18T20:55:33Z

docs/configuration.md

 `hedera.mirror.rosetta.db.host`                         | 127.0.0.1               | The IP or hostname used to connect to the database
 `hedera.mirror.rosetta.db.name`                         | mirror_node             | The name of the database
-`hedera.mirror.rosetta.db.password`                     | mirror_rosetta_pass     | The database password the processor uses to connect.
+`hedera.mirror.rosetta.db.password`                     | mirror_rosetta_pass     | The database password the processor uses to connect
+`hedera.mirror.rosetta.db.pool.maxIdleConns`            | 20                      | The maximum number of idle database connections


nit: would prefer the full Connections vs Conns here and below

Signed-off-by: Xin Li <xin.li@hedera.com>

sonarcloud · 2021-05-19T16:02:05Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
3.9% Duplication

Nana-EC

LGTM

steven-sheehy

LGTM

xin-hedera added 11 commits May 10, 2021 15:21

rosetta-sdk-go v0.6.10

b948fc8

Signed-off-by: Xin Li <xin.li@hedera.com>

Merge remote-tracking branch 'origin/master' into latest-rosetta-sdk

676d9f5

Signed-off-by: Xin Li <xin.li@hedera.com>

rosetta-cli check:construction

9d4fa5b

Signed-off-by: Xin Li <xin.li@hedera.com>

Merge remote-tracking branch 'origin/master' into latest-rosetta-sdk

b237458

Signed-off-by: Xin Li <xin.li@hedera.com>

add construction validation to rosetta-api workflow

df534b3

Signed-off-by: Xin Li <xin.li@hedera.com>

better format

b624983

Signed-off-by: Xin Li <xin.li@hedera.com>

fix github action workflow

fb11a2a

Signed-off-by: Xin Li <xin.li@hedera.com>

use slurpfile array's first element

2cba20f

Signed-off-by: Xin Li <xin.li@hedera.com>

replace all assert.IsType for types.Error check

d30638c

Signed-off-by: Xin Li <xin.li@hedera.com>

add wait_for_balance

4e9a1b5

Signed-off-by: Xin Li <xin.li@hedera.com>

check balance of the account used to run construction validation

4389db1

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera added enhancement Type: New feature P2 rosetta Area: Rosetta API labels May 12, 2021

xin-hedera added this to the Mirror 0.34.0 milestone May 12, 2021

xin-hedera requested a review from a team May 12, 2021 20:23

xin-hedera self-assigned this May 12, 2021

xin-hedera added 2 commits May 13, 2021 09:20

reformat

383994c

Signed-off-by: Xin Li <xin.li@hedera.com>

consistent naming

99c3c2e

Signed-off-by: Xin Li <xin.li@hedera.com>

steven-sheehy requested changes May 13, 2021

View reviewed changes

Nana-EC reviewed May 14, 2021

View reviewed changes

Nana-EC requested changes May 14, 2021

View reviewed changes

xin-hedera added 7 commits May 17, 2021 10:57

validate both data and construction APIs in a single workflow job

952ab79

Signed-off-by: Xin Li <xin.li@hedera.com>

- fix genesis block returning transactions not after genesis timestamp

fc465d3

- fix run-validation always install rosetta-cli Signed-off-by: Xin Li <xin.li@hedera.com>

- upgrade to gorm v1.21.10

e8ef860

- add db pool config Signed-off-by: Xin Li <xin.li@hedera.com>

fix bug caused by gorm v2 breaking changes about ErrRecordNotFound

847bac5

Signed-off-by: Xin Li <xin.li@hedera.com>

remove debug code

f683234

Signed-off-by: Xin Li <xin.li@hedera.com>

remove unnecessary db connection cleanup

49fd6b4

Signed-off-by: Xin Li <xin.li@hedera.com>

address review feedback

f16b057

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera commented May 18, 2021

View reviewed changes

xin-hedera added 2 commits May 18, 2021 13:26

format, fix configuration.md

213933c

Signed-off-by: Xin Li <xin.li@hedera.com>

- address code smells

6d0a014

- refactor errors - increase validation timeout to 20 mins Signed-off-by: Xin Li <xin.li@hedera.com>

Nana-EC requested changes May 18, 2021

View reviewed changes

address feedback

4e88b9c

Signed-off-by: Xin Li <xin.li@hedera.com>

xin-hedera requested a review from Nana-EC May 19, 2021 16:14

Nana-EC approved these changes May 19, 2021

View reviewed changes

steven-sheehy approved these changes May 19, 2021

View reviewed changes

steven-sheehy modified the milestones: Mirror 0.34.0, Mirror 0.35.0 May 19, 2021

xin-hedera merged commit 1307d96 into master May 19, 2021

xin-hedera deleted the latest-rosetta-sdk branch May 19, 2021 21:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to latest rosetta sdk #1975

Upgrade to latest rosetta sdk #1975

xin-hedera commented May 12, 2021 •

edited

Loading

codecov bot commented May 12, 2021 •

edited

Loading

Nana-EC left a comment

Nana-EC left a comment

Nana-EC May 14, 2021

xin-hedera May 18, 2021

Nana-EC May 14, 2021

Nana-EC May 14, 2021

xin-hedera May 18, 2021

Nana-EC May 18, 2021

xin-hedera May 18, 2021 •

edited

Loading

xin-hedera May 18, 2021

xin-hedera May 18, 2021

xin-hedera May 18, 2021

xin-hedera May 18, 2021

xin-hedera May 18, 2021

xin-hedera May 18, 2021

Nana-EC left a comment

Nana-EC May 18, 2021

xin-hedera May 18, 2021

sonarcloud bot commented May 19, 2021

Nana-EC left a comment

steven-sheehy left a comment


		if (./rosetta-cli 2>&1 \| grep -q 'CLI for the Rosetta API'); then
		if (./rosetta-cli 2>&1 \| grep 'CLI for the Rosetta API' > /dev/null); then

		echo "Installing Rosetta CLI"
		curl -sSfL https://raw.githubusercontent.com/coinbase/rosetta-cli/master/scripts/install.sh \| sh -s -- -b .

Upgrade to latest rosetta sdk #1975

Upgrade to latest rosetta sdk #1975

Conversation

xin-hedera commented May 12, 2021 • edited Loading

codecov bot commented May 12, 2021 • edited Loading

Codecov Report

Nana-EC left a comment

Choose a reason for hiding this comment

Nana-EC 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

xin-hedera May 18, 2021 • 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

Choose a reason for hiding this comment

Nana-EC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented May 19, 2021

Nana-EC left a comment

Choose a reason for hiding this comment

steven-sheehy left a comment

Choose a reason for hiding this comment

xin-hedera commented May 12, 2021 •

edited

Loading

codecov bot commented May 12, 2021 •

edited

Loading

xin-hedera May 18, 2021 •

edited

Loading