Conversation
- bump from 2 spaces to 4 spaces
- adds support for AIP-11 - updates code style, commenting, and formatting. - adds missing licenses in source and header files. - updates naming conventions. - modernizes method definitions in source files. - avoids raw loops in favor of standard lib methods. - improves common type-object initialization. - improves feePolicy handling. - improves Message class logic. - adds a UintToString helper method. - adds Uint byte unpacking helper methods. - completely replaces classes within the transactions folder. - adds a test helper method to compare arrays across platforms. - adds a standard base58 method for Ipfs hash parsing. - adds unit tests for new methods - adds unit test fixtures for v2 Transaction types.
- migrates move calls to use std::move except for Uint packing.
current ESP8266 PlatformIO test builds throw errors in CI because the binary won't fit on the device. This only affects CI, as 8266 uses separate instances for live testing.
- split transaction map and json serialization off on their own. - use extraction to reduce map/json method complexity. - create constants for labels and sizes. - improve calculation of Json serialization capacity. - rename 'getMap()' to 'addToJson()' asset methods. - updates tests to reflect changes.
- add macros for packing `uint16`, `uint32`, and `uint64` types into a byte array. - rename `unpack.h` -> `packing.h`. - update headers includes in src files. - rename `test/unpack.cpp` -> `packing.cpp`. - update 'test/CMakeFiles.txt`. - update unit tests.
Codecov Report
@@ Coverage Diff @@
## develop #198 +/- ##
========================================
Coverage ? 100%
========================================
Files ? 61
Lines ? 1181
Branches ? 0
========================================
Hits ? 1181
Misses ? 0
Partials ? 0
Continue to review full report at Codecov.
|
|
@vasild I agree, if it can’t serde, it’s not a good candidate for support. It’s not so much a matter of whether a platform can support serde though;
The library itself doesn't take up much space and is fairly well-oriented towards embedded.
I’d like to eventually find a way of optimizing such that storage is improved, but testability isn’t affected. @ciband has helped improve this situation by breaking up tests so that different aspects can be tested independently. I think this should be good enough in the mean time. I’m leaving this out of the AIP11 feature as it’s not directly concerned with the scope here, but will continue looking at the issue separately 👍 |
ciband
left a comment
There was a problem hiding this comment.
I just did some spot checking and made some comments about general patterns and practices.
- `...(const uintX_t)` -> `(uint8_t)`. - `...(const uintX_t&)` -> `(uint8_t)`.
Detect and improve performance and memory safety using Sanitization. Done using the following flags: - `ASAN_OPTIONS=detect_leaks=1 ` - `-fsanitize=address,undefined,float-divide-by-zero,unsigned-integer-overflow` - `-fno-sanitize-recover=all` - `-fno-optimize-sibling-calls` - `-fsanitize-address-use-after-scope` - `-fno-omit-frame-pointer` - `-g` - `-O1` Changes: - improve signature memory initialization and handling. - `std::move` -> `std::copy` and `std::copy_n` to avoid misusing non-owned values. - change some types to avoid bad comparison and invalid shifts. - refactor Hex Tools to avoid undefined behavior. - refactor some test cases to avoid dangling ptrs. - use `constexpr` for global vars where possible. - `static const` -> `const` for non-literal globals. - update docs and examples to reflect these changes.
- add `id` member to `TransactionData`. - add calls to set txId from `sign`, `serialize`, and `deserialize`. - move txId mapping call to Mapping class. - add tests to maintain coverage.
- add additional security by zeroing created keypairs during the sign/2nd sign logic.
- split mapping into separate mapping and json files. - move json and mapping to a new mapping directory. - create a separate file for shared mapping labels. - rename json and mapping intern static methods to make intent more clear. - rename `Mapping::getMap` -> `Mapping::fromTransactionData`. - rename `Mapping::toJson` -> `Json::fromTransactionMap`. - cleanup json and mapping comments. - change json logic to display txId before signatures. - update unit tests to reflect any changes.
Several transaction-type models use the longer-form `std::array` pattern. A few predefined array-types are also already available in `interfaces/identities` to improve readability and intent. The following were updated within the transaction asset classes: - `std::array<uint8_t, HASH_32_LEN>` -> `Hash32` - `std::array<uint8_t, ADDRESS_HASH_LEN>` -> `AddressHash` The asset types being updated: * `htlc_lock` * `htlc_claim` * `htlc_refund` * `transfer` These changes are non-breaking.
Summary
Implement AIP-11 Support.
Resolves #7 and #90.
move->copy/copy_nsemantics to prevent unwanted modification of sources.typedef->usingsemantics for types .0.2.1->0.3.2.Checklist
Additional Comments
Breaking changes: