Skip to content
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

Support RPC 0.7.0 #162

Merged
merged 15 commits into from
Mar 27, 2024
Merged

Support RPC 0.7.0 #162

merged 15 commits into from
Mar 27, 2024

Conversation

DelevoXDG
Copy link
Collaborator

@DelevoXDG DelevoXDG commented Mar 18, 2024

Stack

-- Support RPC 0.7.0 (#162)
-- Add execute methods with custom fee multipliers (#163)
-- Add transaction version enum (#164)
-- Refactor receipts (#165)

Describe your changes

Add support for RPC 0.7.0

  • Update StarknetExecutionResources; Add StarknetComputationResources
    • Add executionResources to tx traces
    • Add computationResources to StarknetFunctionInvocation
  • Fee-related changes
    • Add dataGasConsumed and dataGasPrice to StarknetFeeEstimate
    • Update maxAmount calculation logic in toResourceBounds()
    • Use multipliers instead of overheads in fee calculation utils
    • Increase multipliers used in toResourceBounds() by default; Use 1.5 multiplier (analogous to 0.5 overhead)
    • Add executeV1, executeV3 methods in StarknetAccount with estimate fee multipliers
  • Introduce StarknetTransactionVersion enum; Use it instead of raw Felt values
  • Merge receipt structs with block info (a.k.a processed) and without it (a.k.a pending)
    • Remove StarknetPendingTransactionReceipt, StarknetProcessedTransactionReceipt protocols
    • Add optional blockHash and blockNumber to StarknetTransactionReceipt protocol and all derived Starknet[..]TransactionReceipt structs

Other

Linked issues

Closes #148
Closes #147

Breaking changes

  • This issue contains breaking changes
  • StarknetFeeEstimate extension fee calculation utils
    • toResourceBounds() now takes amountMultiplier, unitPriceMultiplier instead of amountOverhead, unitPriceOverhead
    • toMaxFee() now takes multiplier instead of overhead
  • version of StarknetTransaction is now of type StarknetTransactionVersion instead of Felt; This applies to all derived transaction structs
  • Removed StarknetPendingTransactionReceipt, StarknetProcessedTransactionReceipt protocols
  • Renamed all Starknet[..]TransactionReceipt protocols to Starknet[..]TransactionReceiptProtocol
  • Removed all StarknetPending[..]TransactionReceipt and StarknetProcessed[..]TransactionReceipt classes; Use Starknet[..]TransactionReceipt classes with optional blockHash and blockNumber instead

- Add `dataAvailability` to `StarknetExecutionResources`; Add `StarknetDataAvailability
  - Add `executionResources` to tx traces
- Add `StarknetComputationResources`
  - Add `computationResources` to `StarknetFunctionInvocation`
- Add `StarknetResources` protocol with common fields
- Prefund account with `5_000_000_000_000_000`->`5_000_000_000_000_000_000`
- Change max fee in declare methods to `1_000_000_000_000_000`->`10_000_000_000_000_000`
- Add `dataGasConsumed`, `dataGasPrice` fields to `StarknetFeeEstimate`
- Update `.toResourceBounds()` extension
  - Update overheads
  - Use new logic
public let type: StarknetTransactionType = .deployAccount

private enum CodingKeys: String, CodingKey {
case validateInvocation = "validate_invocation"
case constructorInvocation = "constructor_invocation"
case feeTransferInvocation = "fee_transfer_invocation"
case stateDiff = "state_diff"
case executionResources = "execution_resources"
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

There was a discussion on Slack that execution_resources should be added to L1HandlerTransactionTrace and it was also mentioned in software-mansion/starknet.py#1312 (comment). Not sure if we want to add it as optional now or wait until it is fixed in the spec.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I thought it would be reasonable not to change this until the spec is updated, but I suppose making it optional won't hurt

DelevoXDG added a commit that referenced this pull request Mar 22, 2024
Copy link
Member

@THenry14 THenry14 left a comment

Choose a reason for hiding this comment

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

lgtm

func toResourceBounds(amountOverhead: Double = 0.1, unitPriceOverhead: Double = 0.5) -> StarknetResourceBoundsMapping {
let maxAmount = addOverhead(self.gasConsumed.value, amountOverhead).toUInt64AsHexClamped()
func toResourceBounds(amountOverhead: Double = 0.5, unitPriceOverhead: Double = 0.5) -> StarknetResourceBoundsMapping {
let maxAmount = self.gasPrice == .zero ? UInt64AsHex.zero : addOverhead(self.overallFee.value / self.gasPrice.value, amountOverhead).toUInt64AsHexClamped()
Copy link
Member

Choose a reason for hiding this comment

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

what are the scenarios for gasPrice equal to zero?

Copy link
Collaborator Author

@DelevoXDG DelevoXDG Mar 26, 2024

Choose a reason for hiding this comment

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

There aren't really any likely scenarios, added these for the same reasons I outlined before:

(0,0) resources can be used in estimateFee / simulateTransactions. It normally shouldn't happen, but I suppose better safe than sorry?

@DelevoXDG DelevoXDG requested a review from THenry14 March 26, 2024 16:54
* Introduce `StarknetTransactionVersion` enum

- Use it instead of `Felt` in tx classes
- Update `TransactionWrapper`

* Reorder cases in `TransactionWrapper`
* Update recepits in line with the spec

- Remove `StarknetPendingTransactionReceipt`, remove it uses from derived classes
- Rename `Starknet[..]TransactionReceipt`->`Starknet[..]TransactionReceiptProtocol`
- Rename `StarknetProcessed[..]TransactionReceipt`->`Starknet[..]TransactionReceiptWithBlockInfo`
- Rename `StarknetPending[..]TransactionReceipt`->`Starknet[..]TransactionReceipt`
- Add `StarknetDeployTransactionReceipt` a.k.a pending deploy receipt
- Update `TransactionReceiptWrapper`

* Merge receipts with and w/o block info

- Remove `StarknetTransactionReceiptWithBlockInfo` protocol
- Add optional `blockHash` and `blockNumber` to `StarknetTransactionReceipt` protocol
- Merge `Starknet[..]TransactionReceiptWithBlockInfo` and `Starknet[..]TransactionReceipt`
  - Remove `StarknetInvokeTransactionReceiptWithBlockInfo`
  - Add optional `blockHash` and `blockNumber` to `Starknet[..]TransactionReceipt`
- Update `TransactionReceiptWrapper`: don't take block info into account
* Use multipliers instead of overheads in fee calculation utils

* Add `executeV1`, `executeV3` methods in `StarknetAccount` with estimate fee multipliers

* Fix `StarknetFeeEstimate.toMaxFee()` docstring

* Add docstrings for `executeV1`, `executeV3` methods

* Update docstrings for `executeVX` with `call(s)` as only argument

- Make them in line with the new methods with multipliers

* Add tests

* Review changes

* Review changes

* Update fee utils docstrings

- Remove percentages
- Remove typo char

Co-authored-by: ddoktorski <45050160+ddoktorski@users.noreply.github.com>

* Remove `executeV1|3` short descriptions; Keep only long ones

* Remove `executeV1|3` short descriptions; Keep only long ones (2)

---------

Co-authored-by: ddoktorski <45050160+ddoktorski@users.noreply.github.com>
@DelevoXDG DelevoXDG merged commit 6da127e into main Mar 27, 2024
1 check passed
@DelevoXDG DelevoXDG deleted the feat/148-support-rpc-0.7.0 branch March 27, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support RPC 0.7.0 Specify transaction version in test cases names
3 participants