Closed bassjobsen closed 6 years ago
@bassjobsen please provide a short summary on what you did. Also include that you changed the API and that this is a breaking pull request 🙂
Please make the title of the pull request more descriptive!
You need to adjust the tests, because currently your pull request breaks the build.
The following line must be reverted. The sidechain transaction have a different structure.
- trs.signature = crypto.signBytes(getDAppTransactionBytes(trs), keys)
+ trs.signatures.push(crypto.signBytes(getChainTransactionBytes(trs), keys))
Thanks 👍
@bassjobsen please adjust the Typescript definition file (index.d.ts
) accordingly
Please remove the following line in transactions/chains.js
. The signatures
array is only available for Mainchain transactions.
+ signatures: []
Good work so far :+1:
okay, i wil try to add that index.d.ts
a.s.a.p, thanks!
@sqfasd @liangpeili This pull requests introduced a BREAKING CHANGE and I would like to discuss this if its Ok to merge.
The API interface changes from:
aschJS.dapp.createDApp
-> aschJS.chain.createChain
aschJS.dapp.createInnerTransaction
-> aschJS.chain.createInnerTransaction
According to Semantic Versioning we would need to increase our asch-js version number from 1.4.2
-> 2.0.0
. Or we could add a proxy file dapp.js
that calls the new functions in chain.js
and so slowly deprecate the functions in dapp.js
. Then we only would need to upgrade version number from 1.4.2
-> 1.5.0
.
If we merge this code as it is it would break the following ASCH repos:
dapp.createInnerTransaction
function will breakmaster
branch from github (https://github.com/AschPlatform/asch-js/tarball/master
)@sqfasd , @liangpeili what would be your preferred option?
@a1300 Can you fix this conflict?
@liangpeili yes, I will do that.
@liangpeili this PR is ready for merging :slightly_smiling_face:
rename
dapp
tochain
and fixed the tests according it