Backend cleanup ( adapter removal )

[bluesign]

trafficstars

Closes #???

Removed the adapter, as it lost its purpose over time. This should increase our test coverage in the long run.

---

For contributor use:

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

[codecov-commenter]

Codecov Report

Merging #220 (2fabb6f) into master (2581481) will increase coverage by 1.39%. The diff coverage is 43.90%.

@@            Coverage Diff             @@
##           master     #220      +/-   ##
==========================================
+ Coverage   51.21%   52.61%   +1.39%     
==========================================
  Files          29       28       -1     
  Lines        2341     2296      -45     
==========================================
+ Hits         1199     1208       +9     
+ Misses        997      942      -55     
- Partials      145      146       +1     

Flag	Coverage Δ
unittests	52.61% <43.90%> (+1.39%)	:arrow_up:

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

Impacted Files	Coverage Δ
server/backend/backend.go	47.71% <39.47%> (-1.25%)	:arrow_down:
server/grpc.go	50.00% <100.00%> (-1.86%)	:arrow_down:
server/rest.go	41.66% <100.00%> (ø)
blockchain.go	75.16% <0.00%> (+0.25%)	:arrow_up:
accountStorage.go	100.00% <0.00%> (+24.00%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[devbugging]

@bluesign actually updating this into CLI made me realize that we probably shouldn't change the types of the backend. It's ok to remove the adapter as that's not used anywhere but changing types to flow-go types break any integration of the emulator. Also, I think returning SDK types makes it easier to use. Do you have some reasons why you would like to keep them flow-go types?

[bluesign]

@sideninja what was the breaking point when you were trying to integrate into CLI ? Somehow CLI uses backend ?

edit: OK, I got some context after a little checking; now the problem is we are a bit in a tangled mess, to be honest, cleaning one side became pushing dirt to the other.

We have address on flow-go, flow-go-sdk, cadence, flowkit etc (same stuff with blocks, identifiers etc ) So it became a bit of converting madness. Maybe we should make an effort on this with all parties to clean up a bit.

I am sorry I totally missed this side effect, but merging this will move all this backend adapter code to flowkit which doesn't make any sense.

[devbugging]

I am sorry I totally missed this side effect but merging this will move all this backend adapter code to flowkit which doesn't make any sense.

Exactly. And it's me who missed it.

I agree I think that is some tech debt we could address by cleaning and unifying some models, but honestly, I wouldn't hold my breath as there are other teams I don't think will have time to prioritize this.

I think we could still remove the adapter part but just keep the backend returning SDK types.

Flowkit and Go SDK should have uniform models, if you find some that are not I can address that.

[bluesign]

@sideninja, my go knowledge is a bit limited, is it possible to make two structs somehow compatible in a 3rd one?

I mean, something like SDK.address can be also accepted as Flow.address

[devbugging]

You can compose structs but the types won't match, the only way to my knowledge of doing anything like it is defining an interface for Address, which arguably could be a nice way of accepting any of those types since they all implement the basic methods.

[bluesign]

Ok I have looked a bit more;

Now seems layers as follows: Emulator > Backend > FlowKit and Emulator > Backend > BackEndAdapter

What would you think about: Emulator ( already using flow-go-sdk types ) > FlowKit and Emulator > Backend ( using flowgo types to be compatible with AN.API ) ?

Consumers can use directly Emulator from emulator, instead of using backend I guess. As is backend seems only to proxy to emulator methods (except EnableAutoMine ) which I think can be moved to emulator easily.

[devbugging]

There is some extra implementation for the events in the backend https://github.com/onflow/flow-emulator/blob/master/server/backend/backend.go#L435-L560

It might be sensible to move that into the emulator, but I'm not in the right mindset now (would have to dig through code to get in the right context) to really decide that.

I feel that backend is also a weird naming for it after. Also the logging not sure if it makes sense to be in that layer.