Strong typing for Stellar #1755
Conversation
|
This is waiting for an internal review and presumably will be merged in September. Please hold off on implementing the mentioned issues, it is currently unclear if we would accept such PRs. |
Thank you for your quick reply, I will discuss with you before implementation. |
|
How to update our firmware should this merge to main? |
core/src/trezor/crypto/base32.py
Outdated
| @@ -1,4 +1,4 @@ | |||
| # Base32 implementation taken from the micropython-lib's base64 module | |||
| # Base32 implementation taken from the micropython-lib'data base64 module | |||
matejcik
force-pushed
the
matejcik/stellar-typing
branch
from
fa8d2fa
to
e5b78e6
Compare
|
rebased, fixed conflict with master. this is now waiting for input from Connect side |
|
TODO:
|
matejcik
force-pushed
the
matejcik/stellar-typing
branch
from
e5b78e6
to
3969ef7
Compare
matejcik
force-pushed
the
matejcik/stellar-typing
branch
2 times, most recently
from
bd340a5
to
7ae7337
Compare
BREAKING CHANGE: StellarAssetType is renamed to StellarAsset. The name StellarAssetType is reused for the corresponding enum. Enums are introduced in several other places. Their values correspond to allowed values of (previously int) fields so this should not pose a compatibility problem. Many fields are now required. We believe that this should not pose a compatibility problem, because all known interfaces to Stellar signing actually accept Stellar XDR on input, whose required fields match the protobuf schema.
[no changelog]
[no changelog]
- removed boilerplate - shortened some names - dropped distinction between v0 and v1 tests because there's now no XDR parsing involved - shortened test bodies to check only fields relevant to the particular test case [no changelog]
[no changelog], covered in Stellar refactor changelog entries
matejcik
force-pushed
the
matejcik/stellar-typing
branch
2 times, most recently
from
31ea244
to
2a854eb
Compare
All test cases are generated and verified in Stellar Laboratory. Source XDR is also tested to match the vectors, and it is easy to verify in Laboratory directly.
matejcik
force-pushed
the
matejcik/stellar-typing
branch
from
2a854eb
to
e1ee9e7
Compare
|
@matejcik anything that can/need to be checked on our side ? |
|
is Stellar Account Viewer in your scope? if yes, then general operation of Stellar |
|
in particular please check that #1699 is fixed |
|
@matejcik Stellar is not in our scope, |
Changes most fields in Stellar protobuf to
required. This mostly matches the Stellar XDR, so it should not introduce incompatibilities on caller sides.The notable exception to "mostly matches" are the timebounds fields. In Stellar XDR, they are optional, and in the past, we would treat them as such. This PR makes timebounds required, and so rejects transactions without timebounds.
Per stellar/js-stellar-base#438 omitting timebounds is strongly discouraged by documentation and existing implementations, so I feel that we do not need to support this feature.
Incidentally, this fixes #1699
Several fields (
memo_type,signer_type,asset.type) now use enums. There is another incompatibility:StellarAssetTypeis renamed toStellarAsset, so thatStellarAssetTypecan be used as the name of the enum.Enum values are binary-compatible with the previously existing fields.
ping @szymonlesisz or @matejkriz for the above, which require modifications in Connect:
timebounds, it must be rejectedStellarAssetTypetoStellarAssetas appropriate. Or let me know if this is a bigger problem and we'll figure out some other way to name this.all testcases have been regenerated with timebounds, and signatures checked against Stellar Laboratory. I renamed the testcases, which unfortunately breaks the UI diff.
fixes #2