shapeshift / web

ShapeShift Web
https://app.shapeshift.com
MIT License
157 stars 180 forks source link

feat: multi-hop step accessor safety #6791

Closed gomesalexandre closed 2 weeks ago

gomesalexandre commented 2 weeks ago

Description

Follows-up on https://github.com/shapeshift/web/pull/6765 with getHopByIndex introducing hop access logic consolidation and safety:

Pull Request Type

Issue (if applicable)

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types or contract interactions might be affected by this PR?

Very low, this should come as no runtime change, do a small regression of swapper end-to-end as paranoia

Testing

Engineering

Operations

Screenshots (if applicable)

gomesalexandre commented 2 weeks ago

I'm currently not able to get quotes on this branch, tried both UTXOs and EVMs, with single and cross-account trades.

@0xApotheosis as discussed, way too much "safety" in this PR indeed, voiding very valid accessors at runtime. Fixed in 74ad060060 - and also removed the whole generics dance while at it. The whole if trade quote is defined, hop is defined was actually invalid.

This prompted us to "fix" types in c437ee2da686cdb40220e4ec464f2dd3093eee2 - note fix in quotation marks here as most of this is really explicitly checking for undefineds - or adding non-null assertion for first and last hops - which we were not doing before.

Confirmed single hop quotes are happy again:

image

And so are multi-hops:

image

Also did a full e2e regression test of a single-hop:

https://github.com/shapeshift/web/assets/17035424/27b9faa3-cd84-4b5a-9c89-94ba1ad15550

And of a multi-hop:

image