Simplify transaction builder 3 #1736
Conversation
davidyuk
force-pushed
the
feature/simplify-tx-types
branch
5 times, most recently
from
76fa5e3
to
aa375e0
Compare
Codecov ReportBase: 82.99% // Head: 82.48% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1736 +/- ##
===========================================
- Coverage 82.99% 82.48% -0.52%
===========================================
Files 84 85 +1
Lines 2947 2946 -1
Branches 563 573 +10
===========================================
- Hits 2446 2430 -16
- Misses 238 245 +7
- Partials 263 271 +8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| export * from './tx/builder'; | ||
| export * from './tx/builder/helpers'; | ||
| export * from './tx/builder/constants'; | ||
| export * from './tx/builder/schema'; | ||
| // TODO: move to constants |
There was a problem hiding this comment.
@davidyuk you may have missed this
There was a problem hiding this comment.
nope, it is definitely to be fixed later
| ): Promise<CtVersion | undefined> { | ||
| if (value != null) return value; | ||
| if (options.consensusProtocolVersion != null) return undefined; | ||
| if (Object.keys(ConsensusProtocolVersion).length === 2) return undefined; |
There was a problem hiding this comment.
Could '2' be in the constants?
There was a problem hiding this comment.
2 means that ConsensusProtocolVersion have a single protocol version, e.g.
{
'Iris': 5,
'5': 'Iris',
}
In this case we don't need to find out the current protocol because it won't be compatible with sdk if it is different what we know.
It can be extracted only as KeysCountInEnumWithASlingleElement, but I would leave it as a TS quirck.
| * @returns Deposit value Buffer. | ||
| */ | ||
| serialize( | ||
| value: Int | undefined, | ||
| options: Parameters<typeof coinAmount['serialize']>[1], | ||
| parameters: Parameters<typeof coinAmount['serialize']>[2], |
There was a problem hiding this comment.
Maybe we could have a type alias with a readable name for such type builds. Parameters<typeof coinAmount['serialize']>[2]
There was a problem hiding this comment.
This is quite verbose, but the base idea is just to copy the signature of the inherited method. I'm doing it frequently with different objects and their parts, so I'm not sure that adding intermediate types would be helpful in all cases, but I'm doing so in the most popular, like this:
Lines 34 to 35 in 6b8888b
BREAKING CHANGE: sync `buildTx` accepts `denomination` in the first argument
```diff
-buildTx({ ... }, { denomination: AE_AMOUNT_FORMATS.AETTOS })
+buildTx({ ..., denomination: AE_AMOUNT_FORMATS.AETTOS })
```
BREAKING CHANGE: `TX_TTL` not exported anymore Use `0` instead.
BREAKING CHANGE: AeSdk.buildTx accepts `tag` in options
Replace `aeSdk.buildTx(Tag.SpendTx, { ... })` with `aeSdk.buildTx({ ..., tag: Tag.SpendTx })`.
BREAKING CHANGE: TX_SCHEMA, TxParamsCommon, TxSchema, TxTypeSchemas not exported anymore
davidyuk
force-pushed
the
feature/simplify-tx-types
branch
from
aa375e0
to
d66c3ae
Compare
based on the work done in #1730
closes #1727
This PR is supported by the Æternity Crypto Foundation