extend API route for get transaction with query parameter withEvents #104
Conversation
process/transactionProcessor.go
Outdated
| if err != nil { | ||
| return UnknownStatusTx, errors.ErrTransactionNotFound | ||
| } | ||
|
|
||
| return string(tx.Status), nil | ||
| } | ||
|
|
||
| func (tp *TransactionProcessor) getTxFromObservers(txHash string, reqType requestType) (*data.FullTransaction, error) { | ||
| func (tp *TransactionProcessor) getTxFromObservers(txHash string, reqType requestType, withEvents bool) (*data.FullTransaction, error) { |
There was a problem hiding this comment.
Please split the logic in this function into two functions, such as one of them does not call alterTxWithScResultsFromSourceIfNeeded or mergeScResultsFromSourceAndDestIfNeeded. This way, it would be trivial to prove (more obvious) that the implementation does never lead to a stackoverflow.
Currently, alterTxWithScResultsFromSourceIfNeeded or mergeScResultsFromSourceAndDestIfNeeded contain some risky recursive calls to getTxFromObserver.
EDIT: sorry, only now I realize that we have getTxFromObserver vs. getTxFromObservers. I will think of some possible refactoring. If you have any ideas, go on :)
| @@ -239,13 +240,19 @@ func GetTransaction(c *gin.Context) { | |||
| return | |||
| } | |||
|
|
|||
| withEvents, err := getQueryParamWithEvents(c) | |||
| if err != nil { | |||
There was a problem hiding this comment.
please insert the error or a more specific one that tells about the parameter
process/transactionProcessor.go
Outdated
| @@ -455,19 +469,46 @@ func (tp *TransactionProcessor) getTxFromObservers(txHash string, reqType reques | |||
| return nil, errors.ErrTransactionNotFound | |||
| } | |||
|
|
|||
| func (tp *TransactionProcessor) getTxWithSenderAddr(txHash, sender string) (*data.FullTransaction, error) { | |||
| func (tp *TransactionProcessor) alterTxWithScResultsFromSourceIfNeeded(txHash string, tx *data.FullTransaction, withEvents bool) *data.FullTransaction { | |||
| if !withEvents && len(tx.ScResults) == 0 { | |||
There was a problem hiding this comment.
maybe change to || instead of && ? I think that if any of the 2 options are false, the tx wouldn't be altered
miiu96
dismissed stale reviews from andreibancioiu and bogdan-rosianu
via
4b98239
Extend API route /transaction/:txHash with a query parameter
?withResults=true.Now when someone wants to get a transaction from proxy has the option to receive also the results of transaction.
receiptor a list ofsmart contract results