Node API hotfixes for Rosetta #4187

andreibancioiu · 2022-06-10T09:31:11Z

On API, return the following extra fields of miniblocks: ProcessingType and ConstructionState.
When account is missing, also return the precise blockInfo when / where the account it's missing.
Populate apiTx.Epoch before the initial fee is computed (workaround)
Adjusted log level for important operations (e.g. starting snapshots).
Improved error handling / error propagation.

codecov-commenter · 2022-06-10T09:41:42Z

Codecov Report

❗ No coverage uploaded for pull request base (feat/rosetta@540ce5c). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head da9a8ae differs from pull request most recent head 3ccf41f. Consider uploading reports for the commit 3ccf41f to get more accurate results

@@               Coverage Diff               @@
##             feat/rosetta    #4187   +/-   ##
===============================================
  Coverage                ?   75.34%           
===============================================
  Files                   ?      634           
  Lines                   ?    82576           
  Branches                ?        0           
===============================================
  Hits                    ?    62214           
  Misses                  ?    15624           
  Partials                ?     4738

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 540ce5c...3ccf41f. Read the comment docs.

…us some refactoring.

bogdan-rosianu · 2022-06-15T09:54:33Z

node/external/transactionAPI/unmarshaller.go

@@ -29,7 +33,13 @@ func (tu *txUnmarshaller) unmarshalReceipt(receiptBytes []byte) (*transaction.Ap
 		return nil, err
 	}

+	receiptHash, err := core.CalculateHash(tu.marshalizer, tu.hasher, rec)


why do you have to calculate the receipt, since you fetch it from storage via a key? That key is the hash of the receipt. Am I missing anything?

The storage key is, actually, the hash of the transaction that generated the receipt. However, moved the receipt hash computation to rosetta, since the "hash of a single receipt" is not meaningful on Node's side.

bogdan-rosianu · 2022-06-15T09:56:18Z

node/external/transactionAPI/apiTransactionProcessor.go

@@ -382,12 +382,13 @@ func (atp *apiTransactionProcessor) castObjToTransaction(txObj interface{}, txTy
 }

 // UnmarshalTransaction will try to unmarshal the transaction bytes based on the transaction type
-func (atp *apiTransactionProcessor) UnmarshalTransaction(txBytes []byte, txType transaction.TxType) (*transaction.ApiTransactionResult, error) {
+func (atp *apiTransactionProcessor) UnmarshalTransaction(epoch uint32, txBytes []byte, txType transaction.TxType) (*transaction.ApiTransactionResult, error) {


a little bit misleading, since I understand from the function's signature that a transaction unmarshals differently in regards to the epoch. this is not true, because you only set the Epoch field. might add a TODO and refactor in another PR with an interface that has populateEpochField(epoch uint32)

right, we need some refactoring here as this function does more than one thing: unmarshal + setting up different things

iulianpascalau · 2022-06-15T10:16:49Z

node/external/transactionAPI/apiTransactionProcessor.go

@@ -382,12 +382,13 @@ func (atp *apiTransactionProcessor) castObjToTransaction(txObj interface{}, txTy
 }

 // UnmarshalTransaction will try to unmarshal the transaction bytes based on the transaction type
-func (atp *apiTransactionProcessor) UnmarshalTransaction(txBytes []byte, txType transaction.TxType) (*transaction.ApiTransactionResult, error) {
+func (atp *apiTransactionProcessor) UnmarshalTransaction(epoch uint32, txBytes []byte, txType transaction.TxType) (*transaction.ApiTransactionResult, error) {


right, we need some refactoring here as this function does more than one thing: unmarshal + setting up different things

iulianpascalau · 2022-06-15T10:17:27Z

node/external/transactionAPI/check.go

@@ -39,6 +39,9 @@ func checkNilArgs(arg *ArgAPITransactionProcessor) error {
 	if check.IfNil(arg.LogsFacade) {
 		return ErrNilLogsFacade
 	}
+	if check.IfNil(arg.Hasher) {
+		return process.ErrNilHasher


unit test for this?

The unit test was in apiTransactionProcessor_test.go, but removed now - hash computation moved to rosetta.

… into rosetta-hotfixes

…led").

…a-hotfixes

…ent).

miiu96 · 2022-06-22T14:01:38Z

node/external/blockAPI/baseBlock.go

+	return bap.getAndAttachTxsToMbByEpoch(miniblockHash, miniBlock, epoch, apiMiniblock, options)
+}
+
+func (bap *baseAPIBlockProcessor) getMiniblockByHash(miniblockHash []byte, epoch uint32) (*block.MiniBlock, error) {


maybe this method can be renamed in getMiniblockByHashAndEpoch

… into rosetta-hotfixes

iulianpascalau · 2022-06-22T14:21:24Z

dblookupext/interface.go

@@ -12,6 +12,7 @@ type HistoryRepositoryFactory interface {
 }

 // HistoryRepository provides methods needed for the history data processing
+// TODO: Move interface where it's needed, not in the package where it's implemented


iulianpascalau · 2022-06-22T14:25:04Z

node/external/blockAPI/interface.go

 	UnmarshalReceipt(receiptBytes []byte) (*transaction.ApiReceipt, error)
+	PopulateComputedFields(tx *transaction.ApiTransactionResult)


iulianpascalau · 2022-06-22T14:27:47Z

node/external/transactionAPI/apiTransactionResults.go

 	}

-	arp.putSmartContractResultsInTransaction(tx, resultsHashes.ScResultsHashesAndEpoch)
+	// Question for review: if there's a receipt, why cannot there by any SCRs, as well?


I do not understand. We might only have receipts without scrs, for instance: execution in the same shard
This does not exclude the case in which we have both receipts and scrs: A calls B that calls C, A and B being 2 contracts in the same shard, C being in another shard

iulianpascalau · 2022-06-22T14:31:23Z

node/external/transactionAPI/errors.go

@@ -19,3 +19,6 @@ var ErrNilFeeComputer = errors.New("nil fee computer")

 // ErrNilLogsFacade signals that the logs facade is nil
 var ErrNilLogsFacade = errors.New("nil logs facade")
+
+var errCannotLoadReceipts = errors.New("cannot load receipt(s)")
+var errCannotContractResults = errors.New("cannot load contract result(s)")


typo: errCannotContractResults => errCannotLoadContractResults

iulianpascalau · 2022-06-22T14:32:20Z

process/block/shardblock.go

@@ -1169,7 +1169,12 @@ func (sp *shardProcessor) updateState(headers []data.HeaderHandler, currentHeade

 		sp.setFinalizedHeaderHashInIndexer(header.GetPrevHash())

-		sp.blockChain.SetFinalBlockInfo(header.GetNonce(), headerHash, scheduledHeaderRootHash)
+		finalRootHash := scheduledHeaderRootHash


iulianpascalau · 2022-06-22T14:32:55Z

process/coordinator/process.go

@@ -17,7 +17,7 @@ import (
 	"github.com/ElrondNetwork/elrond-go-core/data/block"
 	"github.com/ElrondNetwork/elrond-go-core/hashing"
 	"github.com/ElrondNetwork/elrond-go-core/marshal"
-	"github.com/ElrondNetwork/elrond-go-logger"
+	logger "github.com/ElrondNetwork/elrond-go-logger"


"cessary" (unnecessary) alias

iulianpascalau · 2022-06-22T14:34:05Z

state/errors.go

+
+// Error returns the error as string
+func (e *ErrAccountNotFoundAtBlock) Error() string {
+	return fmt.Sprintf("account was not found at block = %d", e.BlockInfo.GetNonce())


provide more info from the blockinfo struct?

AdoAdoAdo · 2022-06-22T14:42:52Z

node/external/blockAPI/shardBlock_test.go

-				Type: block.TxBlock.String(),
+				Hash:              hex.EncodeToString(miniblockHeader),
+				Type:              block.TxBlock.String(),
+				ProcessingType:    "Normal",


can you use the constants instead?
same for below.

AdoAdoAdo · 2022-06-22T14:48:19Z

node/external/timemachine/fee/feeComputer_test.go

-		GasPrice: gasPrice,
+	tx := &transaction.ApiTransactionResult{
+		Epoch: uint32(epoch),
+		Tx: &transaction.Transaction{


TODO: this will also have to change after freeze account feature activation

andreibancioiu added 2 commits June 10, 2022 11:54

Add "receipt.Hash" on API responses.

9f175b3

Temporary workaround to have the "epoch" in the fee computer (API).

136bfdf

andreibancioiu self-assigned this Jun 10, 2022

andreibancioiu changed the title ~~Rosetta hotfixes~~ (do not review yet) Rosetta hotfixes Jun 10, 2022

andreibancioiu added the area:node-api Node API label Jun 10, 2022

andreibancioiu added 4 commits June 14, 2022 14:20

New field: initially paid fee for move balance.

9b1c640

Undo some changes (remove recently added field, not used anymore), pl…

045105e

…us some refactoring.

Merge branch 'feat/rosetta' into rosetta-hotfixes

ebb9ae8

Fix linter etc.

85735fb

andreibancioiu marked this pull request as ready for review June 15, 2022 08:04

andreibancioiu changed the title ~~(do not review yet) Rosetta hotfixes~~ Node API hotfixes for Rosetta Jun 15, 2022

bogdan-rosianu self-requested a review June 15, 2022 09:38

bogdan-rosianu reviewed Jun 15, 2022

View reviewed changes

iulianpascalau self-requested a review June 15, 2022 10:10

iulianpascalau reviewed Jun 15, 2022

View reviewed changes

miiu96 previously approved these changes Jun 15, 2022

View reviewed changes

andreibancioiu added 3 commits June 15, 2022 22:13

On API, add new field: miniblock.IsScheduled.

b6a838c

Merge branch 'rosetta-hotfixes' of github.com:ElrondNetwork/elrond-go…

70ebf71

… into rosetta-hotfixes

Go mod tidy.

344fbfb

andreibancioiu dismissed miiu96’s stale review via 344fbfb June 15, 2022 19:16

andreibancioiu and others added 9 commits June 15, 2022 23:24

Bugfix: populate "IsScheduled".

bd211ea

Merge branch 'feat/rosetta' into rosetta-hotfixes

bb67ed6

For API, set final root hash when scheduled root hash is not available.

49a4a40

On API, for miniblocks, return processing type, construction state.

51c7f89

Return new fields. Fix tests.

a90ea32

Fix setting "finalRootHash" for API.

d395041

Adjust error reporting.

98bf684

Define GetEventsHashesByTxsHashes on history repo.

347e0ff

Recover intra-shard SCRs for processed miniblocks (previously "schedu…

11f4cfb

…led").

andreibancioiu and others added 11 commits June 21, 2022 13:11

Fix getting final roothash.

8dea307

Merge branch 'update-intermediate-self-to-self-miniblock' into rosett…

e92dfa1

…a-hotfixes

Remove workaround.

6682c2d

Merge branch 'feat/rosetta' into rosetta-hotfixes

30ea854

Undo (remove code).

91ec3df

Undo (remove code).

2120c31

Fix after review (part 1, do not compute receipts hash for API).

4836f01

Fix tests. "starting snapshot" changes to logLevel info (important ev…

e14f7ec

…ent).

Fix tests.

7ce8d57

Fix after review: define & export PopulateComputedFields.

3e7664a

Merge branch 'feat/rosetta' into rosetta-hotfixes

c0b085c

andreibancioiu requested review from iulianpascalau, bogdan-rosianu and miiu96 June 22, 2022 13:57

miiu96 reviewed Jun 22, 2022

View reviewed changes

andreibancioiu added 2 commits June 22, 2022 17:24

Fix after review.

990bdb2

Merge branch 'rosetta-hotfixes' of github.com:ElrondNetwork/elrond-go…

8a0eae0

… into rosetta-hotfixes

andreibancioiu requested a review from miiu96 June 22, 2022 14:25

iulianpascalau reviewed Jun 22, 2022

View reviewed changes

Fix after review.

3ccf41f

andreibancioiu requested a review from iulianpascalau June 22, 2022 15:01

miiu96 approved these changes Jun 22, 2022

View reviewed changes

iulianpascalau approved these changes Jun 22, 2022

View reviewed changes

andreibancioiu merged commit c8dabac into feat/rosetta Jun 22, 2022

andreibancioiu deleted the rosetta-hotfixes branch June 22, 2022 16:28

AdoAdoAdo reviewed Jun 23, 2022

View reviewed changes

andreibancioiu added complexity:medium size:medium interested:api-multiversx-com labels Jul 25, 2022

andreibancioiu added the type:improvement label Aug 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node API hotfixes for Rosetta #4187

Node API hotfixes for Rosetta #4187

andreibancioiu commented Jun 10, 2022 •

edited

codecov-commenter commented Jun 10, 2022 •

edited

bogdan-rosianu Jun 15, 2022 •

edited by miiu96

andreibancioiu Jun 22, 2022

bogdan-rosianu Jun 15, 2022

iulianpascalau Jun 15, 2022

iulianpascalau Jun 15, 2022

iulianpascalau Jun 15, 2022

andreibancioiu Jun 22, 2022

miiu96 Jun 22, 2022

iulianpascalau Jun 22, 2022

iulianpascalau Jun 22, 2022

iulianpascalau Jun 22, 2022

iulianpascalau Jun 22, 2022

iulianpascalau Jun 22, 2022

iulianpascalau Jun 22, 2022

iulianpascalau Jun 22, 2022

AdoAdoAdo Jun 22, 2022

AdoAdoAdo Jun 22, 2022

		UnmarshalReceipt(receiptBytes []byte) (*transaction.ApiReceipt, error)
		PopulateComputedFields(tx *transaction.ApiTransactionResult)

Node API hotfixes for Rosetta #4187

Node API hotfixes for Rosetta #4187

Conversation

andreibancioiu commented Jun 10, 2022 • edited

codecov-commenter commented Jun 10, 2022 • edited

Codecov Report

bogdan-rosianu Jun 15, 2022 • edited by miiu96

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreibancioiu commented Jun 10, 2022 •

edited

codecov-commenter commented Jun 10, 2022 •

edited

bogdan-rosianu Jun 15, 2022 •

edited by miiu96