integritee-network / crowdloan

crowdloan status and community contributions
The Unlicense
0 stars 6 forks source link

Tx should happen when InBlock and not wait until Block IsFinalized #95

Closed anizeani closed 2 years ago

anizeani commented 2 years ago

fixes #46 fixes #101 fixes #102 TransactionHandler saves tx when InBlock and not when Finalized. block hash and tx hash are displayed with link to subscan and copy button to copy hash to clipboard. Remarks:

  1. I changed the font-size to 14 and removed the margin from the left of the status text (InBlock/Finalized + block/tx hashes), so that all the text fits in the window. Else, part of the hash is out of the window bound and you need to switch slide to the youtube slide to see the rest of the hash. @brenzi try it out and tell me if it's ok for you.
  2. I guess the copy to clipboard icon could look nicer. @brenzi If you have specific UX remarks, please just specify them and I'll do the change.
  3. ~@brenzi I'm not sure yet, how to catch the tx when it fails (like when liquidity etc.). But I guess this would just be a nice addon and is not necessary for this PR. Therefore I would recommend to add the tx failed handling to a new issue and merge this branch without that feature~
  4. NEW: I have now added an error message to the status bar when a tx failed with its reasoning. Please add your remark if you want the error output differently or any other remark :)
  5. ALSO NEW: I have added the display of the estimated fee. I fixed now a first estimated fee value for 0.1 ksm contribution (its 42.6662 µKSM). I'm not sure if this can change dramatically in the future, but when another value is entered, the estimated fee will be calculated. Is this ok? And just give me general comment about how it looks, I see now that smaller text might not be nice for other info then the transaction hash (should I adapt the font size depending on the output? because tx hash is very long and won't fit to the page if font normal, so I reduced font size)
  6. ~Please validate that the fees are now included correctly in the verification here: https://github.com/integritee-network/crowdloan/blob/when-sending-extrinsic%2C-only-wait-for-InBlock%2C-not-finalized/src/Participate.js#L129-L137 . I think my approach is correct, but a review would be nice.~ Inputting the estimated fee into the calculation of the available balance is somewhat problematic. the reason is, to get the estimate, we make a dummy contribution call "api.tx.crowdloan.contribute(paraId, data.value Math.pow(10, 12), null).paymentInfo(sender)" The problem is now, if somebody enters a large contribution value (above 1000 or 10000 ksm), the dummy call overflows since 10^12 10^4 is too large and the page crashes. There are some ways to circumvent it, but this would mean to only take the estimated fees into consideration if the input value is lower then 10^4... maybe its just enough to show the estimated fees?
brenzi commented 2 years ago

I think we should offer a link to a block explorer, like: https://kusama.subscan.io/extrinsic/0x13d401d96b7c87a9504d680d78e6de7aac7b6277875ac8dbee4fff62eea4b71b

anizeani commented 2 years ago

where do you want to have the link to subscan? after press of the participate button in the same window?

brenzi commented 2 years ago

Where you show the tx hash to the user currently. just make that hash a link

anizeani commented 2 years ago

I implemented the hyperlink to subscan, but subscan can't find the transaction. is it, because it will not get through?

brenzi commented 2 years ago

subscan sometimes lags a little bit. have you tried a minute later? please post tx hashes of your tests here!

brenzi commented 2 years ago

here's the last. maybe add a note that users have to be patient and retry after a minute https://kusama.subscan.io/extrinsic/0xd65752843160f79ebd0f83a5594a0f21653bf061e5699aeef2aa5da65ceed16e

anizeani commented 2 years ago

my latest transactions (not sure about the order): 0xf203ce359e01c0ae96abbe7b0c8b0b3520c726f1ed72ce18a124350a65bd7764 (at 17.22) 0xbb69a21d7ec40996007b3685c97e7aabf01f64cc56078886f460ac68ccae3093 0xce5141f4003b08039050958c3169bc6e9dda4cdfc8581529803158bf0e42fa55 0x210fd95f847c58088effe3c606a4c6d46abdb6850f08285fcf25f842ceafc635 I see that the contributions do happen, but the transaction hash I get is not the extrinsic hash shown in subscan. Extrinsic hashes are: 0x39abefae1f1fc1c03a7b3f9ae3a0e55f4ea2bf58a9f23f67560eb62ca302b034 (at 17.22) 0xe8ba7e1f02299d8d968af18933e841e74afee405da91632de9d529d2becca69e 0x029aa0076169e002a0a039bfdfaba5d48f58ed075cffcdd1ef62a5090fe222fc 0xe6fcf9f6f2796dc7bf68c20b5a45fd60d0daf0c39cd3edd7be6b103301d44da4

anizeani commented 2 years ago

@brenzi, I solved it. We didn't actually output the transaction hash, but the block hash. Of course, the block hash doesn't (and will never) match the actual transaction hash. I now get the signedBlock from the block hash and iterate over all its' extrinsics and I see, that it's the last entry:

const blockHash = status.asInBlock.toString();

// returns SignedBlock
const signedBlock = await api.rpc.chain.getBlock(hash);

// the hash for each extrinsic in the block
console.log(`"ALL EXTRINSICS OF BLOCK${signedBlock}`);
signedBlock.block.extrinsics.forEach((ex, index) => {
  console.log(index, ex.hash.toHex());
});
anizeani commented 2 years ago

@brenzi, it's working for me now, please test. Also tell me, if you still want to see the block hash, or only the tx hash

brenzi commented 2 years ago

moreover: with my first attempt, the extrinsic failed but it was finalized. is there a way how we can tell the user that the extrinsic has failed? You can easily test this right now, because the last few contributions have failed with LiquidityRestrictions https://kusama.subscan.io/extrinsic/0x968fd206fcc6c6a401ed1fe05d49c87cebff988ecabea8c8fe28e4c20912d369

brenzi commented 2 years ago

and please test ASAP, before we merge #97 as this will prevent the error from happening

brenzi commented 2 years ago

the link to subscan and the tx hash are wrong!!

I send from the demo account EijCociWDFh6ZBKY3P6KnvujkmcttiNVrTLS8WvcQ7KDHRx

but the link points to another extrinsic by another address:

The crowdloan page shows me: link: https://kusama.subscan.io/extrinsic/0x15168b95a3f0ce3675d12464807a1b984daa1ab7003e56fd59dba74e0825bc97 tx hash: 0x15168b95a3f0ce3675d12464807a1b984daa1ab7003e56fd59dba74e0825bc97 block hash: 0xe0bb0d06afec6879c648b715ec8dd5c8e7ba6ca3a48668de1efc4b20f39e7c1b

should be: https://kusama.subscan.io/extrinsic/0x6aa8ac9d08011f62019849e57ac16ffb83082be98a76b36d35cd7ec46a126921

block hash is correct, but it seems you just take the last extrinsic in that block, not the correct one

mosonyi commented 2 years ago

LGTM, except the mobile view. image

As cannot participate on mobile it is not so important.