-
Notifications
You must be signed in to change notification settings - Fork 14
feat: fee token in tx template #964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: fee token in tx template #964
Conversation
fda016a to
dcee210
Compare
727fbcb to
ab8cdcf
Compare
ab8cdcf to
a9d458a
Compare
b0c3c18 to
ca29709
Compare
578d9ba to
c23972a
Compare
55ff9ca to
dfddf95
Compare
ca29709 to
60733a2
Compare
825e5cb to
23e3b23
Compare
01a56f5 to
29238ac
Compare
73dffe2 to
fb8aefc
Compare
4a37863
4a37863 to
1db9f33
Compare
1db9f33 to
0cc6faf
Compare
| tokenInfo: { | ||
| name: 'Hathor', | ||
| symbol: 'HTR', | ||
| version: TokenVersion.NATIVE, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chore: Use the constants instead of hardcoding this data.
| tokenInfo: { | |
| name: 'Hathor', | |
| symbol: 'HTR', | |
| version: TokenVersion.NATIVE, | |
| }, | |
| tokenInfo: constants.DEFAULT_NATIVE_TOKEN_CONFIG, |
| private static buildFeeHeader(ctx: TxTemplateContext): FeeHeader { | ||
| const entries: IFeeEntry[] = Array.from(ctx.fees.entries()).map(([tokenUid, amount]) => ({ | ||
| tokenIndex: tokensUtils.getTokenIndex( | ||
| ctx.tokens.map(t => ({ uid: t })), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: There is a warning that this header section wasn't tested. Isn't this a critical part of the resulting transaction, or can we skip this testing to a later stage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch. We are using the fee header in the integration tests. I'll see why it's nove flagging as covered :)
9c9a28d
9c9a28d to
4683ca0
Compare
Depends on #858
Motivation
Add the fee, and fee completion api to the transaction template. It will enable developers to create custom transactions without having to add new methods or args to the wallet facade
Acceptance Criteria
addFeeinstruction, and executorSecurity Checklist