Skip to content

Conversation

evias
Copy link
Contributor

@evias evias commented Nov 19, 2019

Added

  • Added networkType parameter for NamespaceId and MosaicId constructors
  • Added keccak unit tests

Fixed

  • Fixed IdGenerator with mosaicId and namespaceId static method now resolving hash algorithm from networkType parameters, defaulted to NetworkType.MIJIN_TEST
  • Fixed IdGenerator.generateNamespacePath to forward networkType parameter

Changed

  • generateNamespaceId method now added to IdGenerator.

@coveralls
Copy link

coveralls commented Nov 19, 2019

Pull Request Test Coverage Report for Build 834

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 828: 0.0%
Covered Lines: 10
Relevant Lines: 10

💛 - Coveralls

@evias
Copy link
Contributor Author

evias commented Nov 20, 2019

default networkType removed; PR should be good to fix namespaceId hash and mosaicId hash generation on keccak using network type to resolve signature schema.

* @param encoded
*
* @param {string} encoded Hexadecimal notation of namespace id
* @param {NetworkType} networkType The network type for hash algorithm resolution
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant

constructor(id: string | number[]) {
constructor(
id: string | number[],
networkType?: NetworkType,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should remove the ? I think

Copy link
Contributor Author

@evias evias Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no its intended ; using ! in else block.

When you instantiate a NamespaceId by number[] you don't need any NetworkType and would be wrong to default it then to something.

When you instantiate by string, you always need the networkType.

@fboucquez
Copy link
Contributor

@evias @rg911

So finally NamespaceId name resolution depends on the hash algorithm/network type?

I have this PR on hold:

https://github.com/nemtech/nem2-sdk-java/pull/165

Should I pick it up?

@fboucquez
Copy link
Contributor

closed not merged.

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.

4 participants