Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

dev: return deployed contract address in get_transaction_receipt api #1216

Merged
merged 32 commits into from
Nov 18, 2023

Conversation

tonypony220
Copy link
Contributor

@tonypony220 tonypony220 commented Oct 23, 2023

Pull Request type

What is the current behavior?

return zeros

Resolves: #1095
Resolves: #1111

What is the new behavior?

  • runtime api exposes new method to get fee disabled config is_transaction_fee_disabled
  • runtime api exposes new method to get transaction from block extrinsics get_index_and_tx_for_tx_hash
  • runtime api refactor get_events just by id fn get_events_for_tx_by_index(tx_index: u64) -> Option<Vec<StarknetEvent>>

get_transaction_receipt changes:

  • get actual_fee from Transfer event if fee not fee_disabled
  • in case DeployAccount transaction calculates contract_address , cause searching it in events is errorprone

Does this introduce a breaking change?

No

Other information

@tonypony220 tonypony220 changed the title dev: return deployed contract address in get_transaction_receipt api api dev: return deployed contract address in get_transaction_receipt api Oct 23, 2023
@tonypony220
Copy link
Contributor Author

I'm not entirely sure about this logic because I need to delve deeper into events. Still, if someone can take a look and share advice, it would be awesome.

@tonypony220 tonypony220 marked this pull request as ready for review October 24, 2023 13:59
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
@tonypony220 tonypony220 requested a review from tdelabro November 4, 2023 14:46
@tonypony220
Copy link
Contributor Author

@EvolveArt can you maybe take a look?

Copy link
Collaborator

@EvolveArt EvolveArt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall lgtm.

We should create an issue to deal with the deploy_account edge case mentionned in the PR's comments.
Maybe in the future we can also find a more efficient way than looking at the events to retrieve this information.

@tdelabro tdelabro added the feature New feature label Nov 9, 2023
crates/pallets/starknet/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/client/rpc/src/lib.rs Outdated Show resolved Hide resolved
@tdelabro
Copy link
Collaborator

@tonypony220 are you still making progresses on this one?

@tonypony220
Copy link
Contributor Author

@tonypony220 are you still making progresses on this one?

Yeap!

@tonypony220 tonypony220 requested a review from tdelabro November 17, 2023 14:03
crates/runtime/src/lib.rs Outdated Show resolved Hide resolved
crates/primitives/fee/src/lib.rs Show resolved Hide resolved
@tdelabro tdelabro merged commit 6cd4fd2 into keep-starknet-strange:main Nov 18, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature
Projects
None yet
4 participants