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

EIP-1559 transaction support #509

Merged
merged 123 commits into from
Apr 15, 2022
Merged

EIP-1559 transaction support #509

merged 123 commits into from
Apr 15, 2022

Conversation

mloit
Copy link
Contributor

@mloit mloit commented Mar 29, 2022

Well here it is, the long awaited support for EIP-1559 (and other new transaction types)

This is a pretty significant rewrite of the EthereumTransaction core to a more modular approach. With this modular approach, adding new transaction types, in the future, will be much easier (if this PR were to be adopted). This PR does contain some breaking changes, out of necessity, in how you interact with an EthereumTransaction object. As a result of the breaking changes, it may make this PR undesirable. I've done my best to add a deprecated shim layer so that legacy code should still compile and work, albeit with deprecated warnings, with little to no modification.

What this PR brings:

Full encode, decode, signing, and verification of Legacy, and EIP-2718 type transaction envelopes.
Currently EIP-2718 defines two transaction types; EIP-2930 and EIP-1559, both are fully supported for encode, decode, signing, and verification.

I've also built on-top of the excellent work already done by Yaroslav and others for EIP-1559 support (now in PR #510) in the Feature/EIP-1559 branch. So included here is the work already done for the new Gas calculations and block management, though that work is still incomplete and not fully integrated. As it stands now EIP-1559 transactions can be created, signed, and sent, but automatic gas calculation is not implemented.

All parts of the library that rely on EthereumTransaction have been updated as necessary to work with the new Transaction Interface. Though any code that works with gas still only understands and works with the legacy values.

Things needed

Testing, Testing... testing!!! While I've added some tests to validate the decode, verification [and by extension encode] more tests are needed. Preferably made by someone other than me. Not because I'm lazy and don't want to do it, but because I am biased by my interpretation of how things should be. Having someone else who has their own independent interpretation of the specs would help to find any errors in implementation. [though I have tested this code on Rinkeby with good success]

Documentation! As always we need to document how things work. I've added a bunch of quick-help style comments in the code to aide in this direction, but more work, and help, here is needed.

Automatic Gas calculation. This really needs to be complete before we can claim full support for EIP-1559.

Creating Transactions

For the most part things work as they did before. The notable change is you use the TransactionOptions structure to pass the gas and other transaction specific parameters to the transaction object.

    /// Universal initializer to create a new EthereumTransaction object
    /// - Parameters:
    ///   - type: TransactionType enum for selecting the type of transaction to create (default is .legacy)
    ///   - to: EthereumAddress of the destination for this transaction (required)
    ///   - nonce: nonce for this transaction (default 0)
    ///   - chainID: chainId the transaction belongs to (default: type specific)
    ///   - value: Native value for the transaction (default 0)
    ///   - data: Payload data for the transaction (required)
    ///   - v: signature v parameter (default 1) - will get set properly once signed
    ///   - r: signature r parameter (default 0) - will get set properly once signed
    ///   - s: signature s parameter (default 0) - will get set properly once signed
    ///   - options: TransactionObject containing additional parametrs for the transaction like gas
    public init(type: TransactionType? = nil, to: EthereumAddress, nonce: BigUInt = 0,
                chainID: BigUInt? = nil, value: BigUInt? = nil, data: Data,
                v: BigUInt = 1, r: BigUInt = 0, s: BigUInt = 0, options: TransactionOptions? = nil) {

TransactionType is the enum that controls the type of transaction that will be created

public enum TransactionType {
    case legacy
    case eip2930
    case eip1559
}

When using a higher level API, you can still create new transaction types by setting type within the TransactionOptions object you pass. Alternatively you can use the applyOptions function with type set to migrate the transaction type before signing and transmitting.

Direct access to the type specific parameters is no longer available. getOptions() and applyOptions() can be used to query or modify any of those parameters. This is how you will need to access all the gasXXX parameters. A given transaction type will ignore any gas parameters it does not support.

That's all for now

I'm sure there will be questions... ask them below. And please test this code, and comment, and help find the bugs.

I'm not going to mark it as a draft PR for the time being, but I consider it still incomplete, though the core functionality is all there. It's a good start, and I wanted to get it out there so people can start working with it to iron out any issues.

Closes #349

mloit added 29 commits March 28, 2022 14:10
…ope of the right transaction type based on the input
commented code left in as it may be useful down the road?
some lint cleanup
Resolvers updated to always resolve. The suggested value is returned if the option is not set, they can only return nil if the suggested value is nil
…ctures to EthereumTransaction

refactored the init to use the EnvelopeFactory and type specific decoding in each envelope
moved the init(from: decoder) for TransactionOptions from Web3+Structures to Web3+Options
… errors into warnings

one error remains with the removal of the Web3.EIP155Signer class, but this is unlikely to be used directly - only case in the repo is a test case
updated the existing tests to use the new api
minor code cleanups
@mloit mloit marked this pull request as draft March 29, 2022 04:52
@mloit mloit marked this pull request as draft April 6, 2022 14:19
@mloit
Copy link
Contributor Author

mloit commented Apr 6, 2022

Reverted some of the changes made previously back to what's in develop in preparation for a bit of a refactor on how I handle the transaction parameters. The code base still works, you will just get a lot of deprecated warnings, as most of what I backed out were the external changes I made within the library to interact with EthereumTransaction in the new way. The mechanism that was used, and the warning tells you to use, is being refactored and will be updated in the next few days.

* develop:
  chore: removed branch from ci.yml
  ci-run: feat/solidity-error-type added to push/branches in ci.yml
  chore: updated xctestplans
  chore: removed redundant file
  chore: removed recovered references
  chore: fixed a few warnings
  chore: avoiding force unwraps; shortened a few expressions
  feat: decoding ABI with solidity error types
@mloit mloit changed the title Experimental EIP-1559 support EIP-1559 transaction support Apr 7, 2022
mloit added 16 commits April 8, 2022 11:19
* develop:
  Fix typo in WebsocketTests comments.
  Fixing Remote tests
  WebSocket tests are unstable on Carthage. Commented out them yet.
  Rename GanacheTests to RemoteTests
  Comment out failing WS remote test.
  Remote tests test plan toggled on
  Disable some remote test temporary.
  Fix Carthage building.
  Rename Remote tests
  Rename chainVersion to MainChainVersion to make it more explicit.
  Move back to Infura provider in remote tests
  Update CONTRIBUTION.md
  Refactoring
  Update CONTRIBUTION.md
  Update CONTRIBUTION.md
  Add CONTRIBUTION.md

# Conflicts:
#	web3swift.xcodeproj/project.pbxproj
* develop:
  Fix EthereumTransaction decoding bug. Fix appropriate test result for such Add test to check whether all transactions in block decodes well.
  Fix percentiles array extension out of range index bug.
  Fixin typos and revert unnecessary code changes.
  Add comments to GasOracle & common methods - Add `BlockNumber` enum to enable strong typing in block number management. - Full test coverage of GasOracle public methods. - Clean GasOracle class.
  Code cleaning & unnecessary changes revert.
  Move back decode([Any].Type) methods
  Fixups - Update percentile arrays method. - fix JSONRPCmethod validation bug -
  Rewrite GasOracle - Implement generic decodeHex methods for T: DecodableFromHex, [T], [[T]]. - Make RPC API `feeHistory` call internal. - Reimplement `tipFeePercentiles` to return array of percentiles of last tips. - Reimplement `baseFeePercentiles` to return array of percentiles of last baseFee. - Rename `bothFeesPercentiles` to return tuple of arrays of percentiles of last both tips and baseFee. - `JSONRPCresponse` now can decode `Web3.Oracle.FeeHistory` - `JSONRPCparams` now can encode `[Double]` - Add `OracleTests`    - `testPretictBaseFee`    - `testPredictTip`
  Rewrite GasOracle - Now it works with feeHistory JRON RPC Ethereum call - Now all method returns percentiles, which user set on object init.
  Move AnyCodingKeys to appropriate place
  Delete unused hexDecode methods for KeyedDecodingContainer.
  Add 2 dimension array decodeHex method. - Rename decodeHex method to swift name convention. - Add FeeHistory struct initializer.
  Implement feeHistory usage for Gas cost prediction - Add method `decodeHex` for `Arrays<T> where T: DecodableFromHex`. - Add feeHistory request. - Add public feeHistory API call - Move GasOracle from `Web3` scope to `web3.Eth` scope (watch out case of first "w") - Change Oracle struct to work with feeHistory API call. - Refactor `JSONRPCmethod` to make it more explicit. - Refactor `InfuraWebsockerMethod` with a styling.
  Add tests Gas Oracle
  Fixes by PR review
  Fixed by PR review.
  Fixes by PR review.
  Add legacy gas price prediction method (pre EIP-1559)
  Revert "Revert "Implement Oracle class""

# Conflicts:
#	Sources/web3swift/Convenience/Decodable+Extensions.swift
#	Sources/web3swift/Web3/Web3+Structures.swift
…nIndex` is decoded using `blockNumber` key
…ombined

* bug-fix/transaction-index:
  Restore bug fixed in #465/#484 that was somehow reverted where `transactionIndex` is decoded using `blockNumber` key
  restore lint cleanup that was reverted somewhere along the way
Updated to include `UInt`
Corrected the return type to be `T` instead if `BigUInt`
@mloit
Copy link
Contributor Author

mloit commented Apr 15, 2022

The failure is due to an implementation error in Oracle for legacyGasFees, specifically gasPriceLegacyPercentiles()

@mloit mloit marked this pull request as ready for review April 15, 2022 06:54
…r EIP-1159 transactions, even though EIP-1159 transactions do not have a `gasPrice` field
@mloit
Copy link
Contributor Author

mloit commented Apr 15, 2022

Found a workaround/fix. It seems some nodes are reporting a gasPrice value in the JSON for EIP-1159 transactions, even though EIP-1159 transactions have no such field. Oracle appears to depend on this, so the workaround is to store this non-standard value still and report it as gasPrice to satisfy Oracle. This really should be corrected for 3.0, and the workaround removed.

@yaroslavyaroslav yaroslavyaroslav merged commit f483098 into web3swift-team:develop Apr 15, 2022
@mloit mloit deleted the experimental/EIP-1559-combined branch April 15, 2022 14:56
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.

Do you support EIP-1559 Transaction?
3 participants