Add BlockHeight to TransactionResult

[emilic]

Closes: #307

Description

Add missing BlockHeight to the TransactionResult. Note that this update is only for the GRPC API call and does not update the HTTP endpoint. Comments in issue refer requiring openapi schema needing updates so new models can be generated, out of scope for this PR.

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the standards mentioned here.
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[sukantoraymond]

LGTM

[codecov-commenter]

Codecov Report

Merging #308 (e9f2dcd) into master (900f9f9) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #308      +/-   ##
==========================================
+ Coverage   65.98%   66.01%   +0.03%     
==========================================
  Files          21       21              
  Lines        1952     1954       +2     
==========================================
+ Hits         1288     1290       +2     
  Misses        543      543              
  Partials      121      121              

Flag	Coverage Δ
unittests	66.01% <100.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
transaction.go	85.84% <ø> (ø)
access/grpc/convert.go	84.83% <100.00%> (+0.11%)	⬆️

---

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 900f9f9...e9f2dcd. Read the comment docs.

[rrrkren]

probably want to add to access/http as well?

[psiemens]

Good catch @rrrkren.

@emilic You'll also want to update this function to include BlockHeight:

flow-go-sdk/access/http/convert.go

Lines 356 to 373 in 900f9f9

	func toTransactionResult(txr *models.TransactionResult, options []cadenceJSON.Option) (*flow.TransactionResult, error) {
	events, err := toEvents(txr.Events, options)
	if err != nil {
	return nil, err
	}
	var txErr error
	if txr.ErrorMessage != "" {
	txErr = fmt.Errorf(txr.ErrorMessage)
	}
	return &flow.TransactionResult{
	Status: toTransactionStatus(txr.Status),
	Error: txErr,
	Events: events,
	BlockID: flow.HexToID(txr.BlockId),
	}, nil
	}

I believe that will require a change to the TransactionResult JSON model here:

flow-go-sdk/access/http/models/model_transaction_result.go

Line 11 in 900f9f9

type TransactionResult struct {

[emilic]

probably want to add to access/http as well?

I mentioned that this is generated from an openapi schema in another project. I made this PR: onflow/flow#1025 to add the block_height to the schema so we can generate it with the Makefile

@sukantoraymond @psiemens

[emilic]

Good catch @rrrkren.

@emilic You'll also want to update this function to include BlockHeight:

flow-go-sdk/access/http/convert.go

Lines 356 to 373 in 900f9f9

	func toTransactionResult(txr *models.TransactionResult, options []cadenceJSON.Option) (*flow.TransactionResult, error) {
	events, err := toEvents(txr.Events, options)
	if err != nil {
	return nil, err
	}
	var txErr error
	if txr.ErrorMessage != "" {
	txErr = fmt.Errorf(txr.ErrorMessage)
	}
	return &flow.TransactionResult{
	Status: toTransactionStatus(txr.Status),
	Error: txErr,
	Events: events,
	BlockID: flow.HexToID(txr.BlockId),
	}, nil
	}

I believe that will require a change to the TransactionResult JSON model here:

flow-go-sdk/access/http/models/model_transaction_result.go

Line 11 in 900f9f9

type TransactionResult struct {

I'll need to make a PR to the flow-go repo as well to update the openapi schema

[psiemens]

This looks good to me.

Thanks for clarifying @emilic. I think it's OK to merge this as-is given that the HTTP doesn't yet support the block height field.