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.

Sign up for GitHub

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

Jump to bottom

fix: data field in CodableTransaction must be passed into the envelope #624

Merged
merged 5 commits into from Nov 10, 2022
Merged

fix: data field in CodableTransaction must be passed into the envelope #624

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ef06fbf
fix: data field in CodableTransaction must be passed into the envelope
JeneaVranceanu Sep 27, 2022
de151ef
Merge branch 'develop' into fix/envelope-data-field
JeneaVranceanu Nov 10, 2022
d745aca
chore: updated docs and removed + minor clean up
JeneaVranceanu Nov 10, 2022
4fc15ad
chore: merged with remote updated branch
JeneaVranceanu Nov 10, 2022
119d23c
chore: intorduced EIP2930Compatible protocol to support easy access t…
JeneaVranceanu Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 4 additions & 10 deletions Sources/Core/Transaction/CodableTransaction.swift
View file
Open in desktop
Expand Up @@ -56,9 +56,10 @@ public struct CodableTransaction {
set { envelope.value = newValue }
}

// MARK: - Ruins signing and decoding tests if tied to envelop
/// any additional data for the transaction
public var data: Data
public var data: Data {
get { return envelope.data }
set { envelope.data = newValue }
}

// MARK: - Properties transaction type related either sends to a node if exist

Expand Down Expand Up @@ -174,8 +175,6 @@ public struct CodableTransaction {
public init?(rawValue: Data) {
guard let env = EnvelopeFactory.createEnvelope(rawValue: rawValue) else { return nil }
self.envelope = env
// FIXME: This is duplication and should be fixed.
data = Data()
noncePolicy = .latest
gasLimitPolicy = .automatic
gasPricePolicy = .automatic
Expand Down Expand Up @@ -233,9 +232,6 @@ extension CodableTransaction: Codable {
public init(from decoder: Decoder) throws {
guard let env = try EnvelopeFactory.createEnvelope(from: decoder) else { throw Web3Error.dataError }
self.envelope = env
// FIXME: This is duplication and should be fixed.
data = Data()

noncePolicy = .latest
gasLimitPolicy = .automatic
gasPricePolicy = .automatic
Expand Down Expand Up @@ -420,8 +416,6 @@ extension CodableTransaction {
chainID: BigUInt = 0, value: BigUInt = 0, data: Data = Data(),
gasLimit: BigUInt = 0, maxFeePerGas: BigUInt? = nil, maxPriorityFeePerGas: BigUInt? = nil, gasPrice: BigUInt? = nil,
accessList: [AccessListEntry]? = nil, v: BigUInt = 1, r: BigUInt = 0, s: BigUInt = 0) {
// FIXME: This is duplication and should be fixed.
self.data = data
self.accessList = accessList
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is WIP but commenting what I see just to make sure. I guess accessList can be removed as well here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also the comments of the function could be cleaned up. As most params are optional but a lot of them still read as (required)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the input.
Next week I'll come back to this PR and will thoroughly check everything you've suggested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Sorry for delay.
A new EIP2930Compatible protocol was introduced to support kind of easy access to accessList.

self.gasLimitPolicy = .automatic
self.noncePolicy = .pending
Expand Down