-
Notifications
You must be signed in to change notification settings - Fork 217
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
Finish integration tests awaiting signTransaction endpoint #2950
Conversation
08e094d
to
5bc7aff
Compare
-- check that reward account is 0, | ||
-- make sure wallet balance is increased by withdrawalAmt - fee | ||
|
||
-- TODO check that reward account is 0 (??) |
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.
Piotr, do you know what this means/how I could check it is true?
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.
Oh, I believe it means just to assert that reward balance on the rewardWallet is 0 after withdrawal.
expectField (#balance . #reward . #getQuantity) (`shouldBe` 0)
(or smth)
-- make sure balance is updated accordingly on src and dst wallets | ||
-- make sure delegation cerificates are inserted | ||
|
||
-- TODO: make sure delegation cerificates are inserted |
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.
As you can see, I've left two TODO items. I'm not sure how to check that delegation certificates are inserted, and that the metadata made it to the chain.
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.
I think that:
- for metadata it should be enough to check that tx status is eventually "in_ledger" and that it contains "metadata" field with the content as expected.
- for delegation it should be enough to check that wallet has delegation -> next active with the pool_id
(I think however that delegation is not working, I've tried it on testnet and with e2e tests yesterday (#2949) - it seems that creating this kind of tx with construct ep does not trigger wallet to delegate)
request @ApiSignedTransaction ctx signEndpoint Default toSign | ||
|
||
-- Submit tx | ||
submitTx ctx signedTx [ expectResponseCode HTTP.status202 ] |
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.
This one fails with:
The submitted transaction was rejected by the local node. Here's an error message that may help with debugging: TxValidationErrorInMode (ShelleyTxValidationError ShelleyBasedEraAlonzo (ApplyTxError [DelegsFailure (WithdrawalsNotInRewardsDELEGS (fromList [(RewardAcnt {getRwdNetwork = Mainnet, getRwdCred = KeyHashObj (KeyHash "89fc61d21ddfcbd4d43652bf05c40c346fa794871423b65052d7614c")},Coin 1000000000000)]))])) AlonzoEraInCardanoMode
I think that means it's a bug in the implementation?
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.
I think that withdrawal from external wallet may not be supported on construct tx ep (https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/constructTransaction) it used to be a thing for old tx ep (https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/postTransaction -> redemption) as a feature to allow users to withdraw funds from ITN wallets.
But if that's the case the construct ep should not allow mnemonics in the first place, so yeah it is a bug in implementation.
Implementing the signing and submission causes the following two tests to fail, indicating a bug/actual issue in the current implementation:
and
These items are tracked in https://input-output.atlassian.net/browse/ADP-1189, and so I will make these tests "pending". |
d78e10f
to
0f29964
Compare
- We'll be adding tests for signing and submission of transactions, so provide helpers to make that easier.
- Fill out TODO items on non-pending tests related to signing and submission.
- Make tests that aren't working atm PENDING. These failures are tracked in ADP-1189.
c53f4fc
to
f5a786d
Compare
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.
LGTM!
bors r+ |
Build succeeded: |
ADP-1185