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.

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

New storer events hashes by transaction hash #2428

Merged
merged 19 commits into from Nov 24, 2020

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Nov 2, 2020

Extend API route/transaction/:hash with a query parameter ?withResults=true in order to can return also the results that are generated by a transaction:

  1. if we have an invalid transaction an receipt will be attached in the transaction
  2. if we have a smart contract call we will have a slice of smart contract results attached in the transaction

This feature will be activated if we have [DbLookupExtensions] Enabled = true

@miiu96 miiu96 self-assigned this Nov 2, 2020
@miiu96 miiu96 changed the title New storer events hashes by transaction hash [WIP do not reveiw] New storer events hashes by transaction hash Nov 3, 2020
Copy link
Contributor

@sasurobert sasurobert left a comment

Choose a reason for hiding this comment

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

Good implementation - Event name does not sound super good. It may be confused by smart contract events and logs. We need an API for those as well - next sprint maybe.

dataRetriever/interface.go Outdated Show resolved Hide resolved
node/nodeTransactionsEvents.go Outdated Show resolved Hide resolved
node/nodeTransactionsEvents.go Outdated Show resolved Hide resolved
process/block/baseProcess.go Outdated Show resolved Hide resolved
return scr, nil
}

func (n *Node) adaptSmartContractResult(scr *smartContractResult.SmartContractResult) *transaction.SmartContractResultApi {
Copy link
Contributor

Choose a reason for hiding this comment

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

scrToscrApi ? and maybe have the same type of function names for all these transformation (rewards, transactions, receipts) and move them to a separate file.

core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
api/mock/facade.go Outdated Show resolved Hide resolved
core/dblookupext/historyRepository.go Outdated Show resolved Hide resolved
core/dblookupext/historyRepository.go Show resolved Hide resolved
core/dblookupext/interface.go Outdated Show resolved Hide resolved
core/dblookupext/proto/eventsHashesByTxHash.proto Outdated Show resolved Hide resolved
core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
core/dblookupext/eventsHashesByTxHashIndex.go Show resolved Hide resolved
core/dblookupext/historyRepository.go Outdated Show resolved Hide resolved
core/dblookupext/proto/eventsHashesByTxHash.proto Outdated Show resolved Hide resolved
sasurobert
sasurobert previously approved these changes Nov 10, 2020
core/dblookupext/epochByHash.pb.go Outdated Show resolved Hide resolved
core/dblookupext/miniblockMetadata.pb.go Outdated Show resolved Hide resolved
core/dblookupext/resultsHashesByTxHash.pb.go Outdated Show resolved Hide resolved
originalTxHash := string(rec.TxHash)
if eventsHashes, ok := eventsHashesMap[originalTxHash]; ok {
// append receipt hash in already create event
eventsHashes.ReceiptsHash = []byte(receiptHash)
Copy link
Contributor

Choose a reason for hiding this comment

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

if it's just 1 receipt then overwriting the receipt on the ok branch would be unreachable code right?

we could then simply create the entry for this hash always and add it to the map without checking for existence first.

core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
core/dblookupext/eventsHashesByTxHashIndex.go Outdated Show resolved Hide resolved
}

eventsHashes, err := eventsHashesIndex.getEventsHashesByTxHash(txWithReceiptHash, epoch)
require.Nil(t, err)
require.Equal(t, expectedEvents, eventsHashes)
}

func TestGroupSmartContractResults(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Before I was referring to directly testing the grouping function, but all right.

Copy link
Contributor

@stefanandy stefanandy left a comment

Choose a reason for hiding this comment

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

System Test Passed

@LucianMincu LucianMincu merged commit a5be998 into development Nov 24, 2020
@LucianMincu LucianMincu deleted the EN-7997-events-hashes-tx-hash branch November 24, 2020 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants