Skip to content

Conversation

@rg911
Copy link
Contributor

@rg911 rg911 commented Aug 8, 2019

Catbuffer

Catbuffer Typescript Generator has been developed currently under reviewing. Opening PR can be found here. All schemas defined in Catbuffer can be generated by this generator except AggregateTransaction, Cosignature and DetachedCosignature. Those schemas are currently manually created. Development is undergoing to patch the generator to support them.

Catbuffer implementation (In this PR)

  • Removed Flatbuffer codes: Transaction builder, schema, buffer codes

  • Added generated Typescript catbuffer codes.

  • Manually patched AggregateTransaction, Cosignature and DetachedCosignature schemas

  • Refactored signWith() and added two abstract methods: generateBytes(), generateEmbeddedBytes() in Transaction.ts.

  • Refactored all transaction models added generateBytes(), generateEmbeddedBytes() using catbuffer generated classes.

  • Refactored all Cosignatories signing methods in AggregateTransaction.ts and CosignatureTransaction.ts

  • Updated some models to match the latest catbuffer schema.

  • Patched the unit tests and e2e tests

  • Refactored TransactionMapping.CreateTransactionFromPayload() to use catbuffer.

    Breaking changes:

    1. MosaicDefinition:
      • Optional properties has been removed from catapult-server.
      • Duration is now required. **For defining eternal mosaic, use Uint64.fromUint(0)
      • Proper mosaicProperty refactoring will be done in stage 2.
      • **Note: current ws listener (for MosaicDefinition) and MosaicHttp is not working properly. liaising with catapult-rest on these issues right now.
    2. 'Action' enums:
      • According to recent catbuffer schema changes, some 'actionType' enum values have been swapped. e.g. Add = 0x00 => Add = 0x01
      • Affected enums: LinkAction, MultisigCosignatoryModificationType, AliasAction, MosaicSupplyType, RestrictionModificationType (AccountRestriction)

Issue: #92

@rg911 rg911 requested a review from evias August 8, 2019 12:16
@coveralls
Copy link

coveralls commented Aug 8, 2019

Pull Request Test Coverage Report for Build 638

  • 2232 of 2582 (86.44%) changed or added relevant lines in 115 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+6.9%) to 67.307%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/core/format/Convert.ts 7 8 87.5%
src/infrastructure/Listener.ts 0 1 0.0%
src/infrastructure/MosaicHttp.ts 0 1 0.0%
src/infrastructure/catbuffer/AccountAddressRestrictionModificationBuilder.ts 21 22 95.45%
src/infrastructure/catbuffer/AccountMosaicRestrictionModificationBuilder.ts 21 22 95.45%
src/infrastructure/catbuffer/AccountOperationRestrictionModificationBuilder.ts 20 21 95.24%
src/infrastructure/catbuffer/AccountRestrictionModificationBuilder.ts 14 15 93.33%
src/infrastructure/catbuffer/AddressDto.ts 11 12 91.67%
src/infrastructure/catbuffer/Hash256Dto.ts 11 12 91.67%
src/infrastructure/catbuffer/KeyDto.ts 11 12 91.67%
Files with Coverage Reduction New Missed Lines %
src/core/utils/DtoMapping.ts 1 26.92%
src/model/UInt64.ts 2 91.67%
Totals Coverage Status
Change from base Build 595: 6.9%
Covered Lines: 5406
Relevant Lines: 7304

💛 - Coveralls

rg911 added 26 commits September 2, 2019 12:02
Appended new signwithCatbuffer method in Transaction (temp)
Applied catbuffer builders on standalone transaction (builder)
Updated unit tests
- createTransactionFromPayload now use catbuffer
- Catbuffer use separate builder class for Complete and Bonded
- Added unit tests
Appended new signwithCatbuffer method in Transaction (temp)
Applied catbuffer builders on standalone transaction (builder)
Updated unit tests
Appended new signwithCatbuffer method in Transaction (temp)
Applied catbuffer builders on standalone transaction (builder)
Updated unit tests
- createTransactionFromPayload now use catbuffer
@rg911 rg911 merged commit 9a6e7e0 into symbol:master Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants