feat(sozo): add --receipt transaction option

[remybar]

Related to DOJ-215, #1627.

Modify the --wait transaction option to show the execution status and the transaction hash only.
Then, add the --receipt transaction option to be used with the --wait flag to show the full
receipt after the transaction execution.

[codecov]

Codecov Report

Attention: Patch coverage is 9.43396% with 48 lines in your changes are missing coverage. Please review.

Project coverage is 69.54%. Comparing base (ae8db10) to head (840b628).
Report is 1 commits behind head on main.

Files	Patch %	Lines
bin/sozo/src/utils.rs	0.00%	27 Missing ⚠️
bin/sozo/src/ops/auth.rs	0.00%	9 Missing ⚠️
crates/dojo-world/src/utils.rs	0.00%	8 Missing ⚠️
bin/sozo/src/ops/register.rs	0.00%	2 Missing ⚠️
bin/sozo/src/commands/options/transaction.rs	0.00%	1 Missing ⚠️
bin/sozo/src/ops/execute.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1647      +/-   ##
==========================================
- Coverage   69.59%   69.54%   -0.06%     
==========================================
  Files         266      267       +1     
  Lines       27312    27339      +27     
==========================================
+ Hits        19009    19013       +4     
- Misses       8303     8326      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kariy]

hmmm i don't quite like having --wait-no-receipt flag to handle this case... much prefer if we have a --receipt flag and choose whether to print out the receipt based on that instead.

@glihm any idea?

[remybar]

hmmm i don't quite like having --wait-no-receipt flag to handle this case... much prefer if we have a --receipt flag and choose whether to print out the receipt based on that instead.

@glihm any idea?

Yes, good idea 👍
But that means the --wait flag won't show the receipt anymore. I think it's not a big deal except if an user uses the actual output to do something :)

[glihm]

hmmm i don't quite like having --wait-no-receipt flag to handle this case... much prefer if we have a --receipt flag and choose whether to print out the receipt based on that instead.

--receipt => 💯

But that means the --wait flag won't show the receipt anymore. I think it's not a big deal except if an user uses the actual output to do something :)

Yup, this is I think the intention of Val, not having too much information by default. Somehow, when polling explicitly a transaction with sozo, it's good to have details. But as the TransactionOptions may be used in a command that already generates several prints, having a short representation by default is better.

[remybar]

hmmm i don't quite like having --wait-no-receipt flag to handle this case... much prefer if we have a --receipt flag and choose whether to print out the receipt based on that instead.

--receipt => 💯

But that means the --wait flag won't show the receipt anymore. I think it's not a big deal except if an user uses the actual output to do something :)

Yup, this is I think the intention of Val, not having too much information by default. Somehow, when polling explicitly a transaction with sozo, it's good to have details. But as the TransactionOptions may be used in a command that already generates several prints, having a short representation by default is better.

Ok, thanks !

For you @glihm and @kariy, to wait and see the receipt we just pass --receipt or we have to pass --wait also ? I mean, is --receipt means "wait and show the receipt" or is it just a flag for the --wait option to show the receipt instead of a short status and hash ?

[glihm]

For you @glihm and @kariy, to wait and see the receipt we just pass --receipt or we have to pass --wait also ? I mean, is --receipt means "wait and show the receipt" or is it just a flag for the --wait option to show the receipt instead of a short status and hash ?

In the current proposal, I would say:

1. User has to pass --wait to explicitly wait the transaction.
2. If --receipt is passed along with --wait, then the receipt is printed.
3. --receipt without --wait will have no effect, at least for now.

[glihm]

Looks good to me, thanks for the update @remybar! Will let @kariy confirming the final review. 👍

[kariy]

In the current proposal, I would say:

1. User has to pass --wait to explicitly wait the transaction.
2. If --receipt is passed along with --wait, then the receipt is printed.
3. --receipt without --wait will have no effect, at least for now.

Yes, this is good.

[kariy]

Looks good. I've added some suggestions on what to print if the --wait flag is set without --receipt.

[remybar]

Hi @glihm and @kariy !

I tried to test with a reverted transaction to see how the log looks like but I was not able to get a reverted transaction 😅

I changed the grant_writer function in world.cairo to call panic! but the error is directly catched there, with the error "Failed to send transaction" ...

dojo/bin/sozo/src/ops/auth.rs

Lines 27 to 31 in 916ee30

	let res = account
	.execute(calls)
	.send()
	.await
	.with_context(|| "Failed to send transaction")?;

I also tried to add a loop before the call to panic! to have a function which takes some times but it does not work.

Any idea ?

[glihm]

Hi @glihm and @kariy !

I tried to test with a reverted transaction to see how the log looks like but I was not able to get a reverted transaction 😅

I changed the grant_writer function in world.cairo to call panic! but the error is directly catched there, with the error "Failed to send transaction" ...

dojo/bin/sozo/src/ops/auth.rs

Lines 27 to 31 in 916ee30

	let res = account
	.execute(calls)
	.send()
	.await
	.with_context(|| "Failed to send transaction")?;

I also tried to add a loop before the call to panic! to have a function which takes some times but it does not work.

Any idea ?

Ah yes.. this is because of the fee estimation.
When we don't give a fix max_fee, starknet-rs is attempting to estimate the fee first. And doing so, the execution fails, and we got an error instead of the transaction hash.

If you want to see the output as expected, you can do that for instance:

let res = account
  .execute(vec![Call {
      calldata,
      to: contract_address,
      selector: get_selector_from_name(&entrypoint)?,
}])
.max_fee(starknet::core::types::FieldElement::from_hex_be("0xfffffffffffffffffffffff").unwrap())
.send()
.await?;

And here you'll have the expected output.

@kariy should we add to the TransactionOptions a max_fee field actually?
@remybar if the estimation of the fee fails, there's no transaction at all, hence the text you've seen with Failed to send transaction.

[remybar]

After some tests, I think the following format is more convenient for all the possible cases. What do you think ?

Without flags:

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f

With --wait:

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f
Status: OK

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f
Status: REVERTED
Reason:
Error in the called contract (0x06162896d1d7ab204c7ccac6dd5f8e9e7c25ecd5ae4fcb4ad32e57786bb46e03):
Error at pc=0:4573:
Got an exception while executing a hint: Hint Error: Requested contract address ContractAddress(PatriciaKey(StarkFelt("0x01385f25d20a724edc9c7b3bd9636c59af64cbaf9fcd12f33b3af96b2452f295"))) is not deployed.

With --wait --receipt:

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f
Receipt:
{
...
}

[kariy]

After some tests, I think the following format is more convenient for all the possible cases. What do you think ?

Without flags:

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f

With --wait:

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f
Status: OK

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f
Status: REVERTED
Reason:
Error in the called contract (0x06162896d1d7ab204c7ccac6dd5f8e9e7c25ecd5ae4fcb4ad32e57786bb46e03):
Error at pc=0:4573:
Got an exception while executing a hint: Hint Error: Requested contract address ContractAddress(PatriciaKey(StarkFelt("0x01385f25d20a724edc9c7b3bd9636c59af64cbaf9fcd12f33b3af96b2452f295"))) is not deployed.

i like this formats, these look good.

With --wait --receipt:

Transaction hash: 0x5ef861cc4b1e451febb614b0792c5ba81d94931df99e1d4ba2841d51e2d1f1f
Receipt:
{
...
}

maybe we don't need to show the tx hash because the receipt already includes it

[kariy]

@remybar there's a conflict now that #1644 is merged

EDIT: wrong PR

[remybar]

maybe we don't need to show the tx hash because the receipt already includes it

I would keep the same format for all the cases, even if the tx hash is already in the receipt. If you want to get the tx hash, you can copy/paste it really quickly whatever the given flags. What do you think ? If you strongly want to remove it when we display the full receipt, I do it :)

Another argument is that when you provide --wait --receipt you immediately see the tx hash and, if the tx takes time, you will see the full receipt some seconds/minutes after.

[kariy]

Another argument is that when you provide --wait --receipt you immediately see the tx hash and, if the tx takes time, you will see the full receipt some seconds/minutes after.

Ah I agree, this does seems nice...

[kariy]

I think this looks good, I just have one last request and then we're good.

[kariy]

nice work, thanks!

[kariy]

@remybar there's conflicts now due to #1576 🙏