Update transaction test using updated protocol-spec - Closes #5368, #5335 and #5209 #5383
Conversation
shuse2
force-pushed
the
5368-update_transaction_test
branch
from
e7bb8b6
to
239e081
Compare
| signData, | ||
| } = require('@liskhq/lisk-cryptography'); | ||
| const { signData } = require('@liskhq/lisk-cryptography'); | ||
| const { Codec } = require('@liskhq/lisk-codec'); |
There was a problem hiding this comment.
Why to use class and not the instance codec?
There was a problem hiding this comment.
I just wanted to make sure there is no duplicate registration of schema
| const BaseGenerator = require('../base_generator'); | ||
|
|
||
| const networkIdentifier = | ||
| 'e48feb88db5b5cf5ad71d93cdcd1d879b6d5ed187a36b0002cc34e0ef9883255'; | ||
| const codec = new Codec(); |
There was a problem hiding this comment.
We have a default instance in the package already, should be used.
| const networkIdentifier = Buffer.from( | ||
| 'e48feb88db5b5cf5ad71d93cdcd1d879b6d5ed187a36b0002cc34e0ef9883255', | ||
| 'hex', | ||
| ); |
There was a problem hiding this comment.
This networkIdentifier is shared across almost all of the generators. Can we move such shared stuff to a common file to reduce the code footprint so we can review and manage it easily.
There was a problem hiding this comment.
yes but with the current structure, all of the generator is independent, and even if we change the network identifier and it still works
| @@ -26,7 +26,8 @@ | |||
| "framework", | |||
| "elements/*", | |||
| "commander", | |||
| "sdk" | |||
| "sdk", | |||
| "protocol-specs" | |||
There was a problem hiding this comment.
With this change, now all elements used in protocol-specs will be linked to local packages. Previously only published packages were used in here. Does that will impact any thing?
There was a problem hiding this comment.
With only lerna, that was the case, but it seems like if we specify the old version, it is fetching the old version. so unless we increase the dependency version it's ok
shuse2
force-pushed
the
5368-update_transaction_test
branch
2 times, most recently
from
2cc0932
to
8846458
Compare
shuse2
requested review from
nazarhussain,
pablitovicente and
ishantiw
and removed request for
pablitovicente
shuse2
force-pushed
the
5368-update_transaction_test
branch
from
c08d2dd
to
6989e34
Compare
shuse2
force-pushed
the
5368-update_transaction_test
branch
from
6989e34
to
6b6297d
Compare
What was the problem?
This PR resolves #5368, resolves #5335 and #5209
How was it solved?
How was it tested?