feat: add contract ID from PanicReceipts #583
Conversation
QuinnLee
changed the title
Coverage report
Show files with reduced coverage 🔻
Test suite run success545 tests passing in 49 suites. Report generated by 🧪jest coverage report action from 3cc7746 |
QuinnLee
force-pushed
the
ql/462
branch
7 times, most recently
from
45d7e2f to
6c47543
Compare
| { type: 1, inputIndex: 0 }, | ||
| { type: 1, inputIndex: 1 }, | ||
| ]); | ||
| const scope = contract.functions.call_external_foo(1336, otherContract.id); |
There was a problem hiding this comment.
I think this is sufficient for tests following the pattern here.
The only issue is that the underlying transaction request doesn't get mutated if there are updates on to it so the getContract assertion is no longer needed. I'm going to create follow-up issue to see if we should update the scope.transactionRequest
QuinnLee
requested review from
LuizAsFight,
luizstacio,
camsjams and
a team
and removed request for
LuizAsFight,
luizstacio and
camsjams
| ): receipt is ReceiptPanic => | ||
| receipt.type === ReceiptType.Panic && | ||
| receipt.contractId !== '0x0000000000000000000000000000000000000000000000000000000000000000'; | ||
|
|
There was a problem hiding this comment.
Nice use of type helpers. ⚡
Since getReceiptsWithMissingOutputVariables and getReceiptsWithMissingContractIds don't actually get a list of receipts, I suggest renaming them using question-like nomenclature.
One literal example would be:
doesReceiptHaveMissingOutputVariablesdoesReceiptHaveMissingContractId
There was a problem hiding this comment.
got it! this makes sense. Will update!
| transactionRequest.addContract(Address.fromString(contractId)) | ||
| ); | ||
| tries += 1; | ||
| } while (tries < MAX_RETRIES); |
There was a problem hiding this comment.
What is the reasoning behind MAX_RETRIES, and why is it 10 and not 20 or 30 or other value?
There was a problem hiding this comment.
TBH, I just used the same number as the Rust SDK set. Not sure what the motivation for it is too. cc @digorithm - should we set a max retry to a different number
TLDR
Add missing contractId's from panic receipts
Follow-up
Figure out if the underlying transactionRequest on the scope should be updated too