web3swift icon indicating copy to clipboard operation
web3swift copied to clipboard

Transaction option is not applied to send translation.

Open Kristenlike1234 opened this issue 2 years ago • 5 comments

There is a problem that the trasactionOptions cannot be used properly because of merge. In the Web3+ MutatingTransaction class, it is replaced by cleanedOptions. I want to know the reason. 2.6.5 library version. Estimategas for EIP1559 is excellent. However, transaction option is not applied to send translation.

var options = writeTransaction.transactionOptions
        options.nonce = .latest
        options.callOnBlock = .latest
        options.type = .eip1559
        options.maxFeePerGas = .manual(maxFee)
        options.maxPriorityFeePerGas = .manual(maxTip)

If you run sendTransactionPromise with the above options, maxFeePerGas and maxPriorityFeePerGas are ignored. There seems to be some problem when merge.

public func sendPromise(password: String = "web3swift", transactionOptions: TransactionOptions? = nil) -> Promise<TransactionSendingResult>{
    let queue = self.web3.requestDispatcher.queue
    return self.assemblePromise(transactionOptions: transactionOptions).then(on: queue) { transaction throws -> Promise<TransactionSendingResult> in
      let mergedOptions = self.transactionOptions.merge(transactionOptions)
      var cleanedOptions = TransactionOptions ()
      cleanedOptions.to = mergedOptions.to
      return self.web3.eth.sendTransactionPromise(transaction, transactionOptions: cleanedOptions, password: password)
    }
}

image

maxFeePerGas , maxPriorityFeePerGas, type, nonce all nil. Only from, to exists.

/// create a JSCON RPC Request object for the given transaction
/// - Parameters:
///   - method: RPC request method
///   - transaction: the thansaction to encode/send
///   - trasactionOptions: additional options for the transaction
/// - Returns: a JSCONRPCrequest object
static func createRequest(method: JSONRPCmethod, transaction: EthereumTransaction, transactionOptions: TransactionOptions?) -> JSONRPCrequest? {
   let onBlock = transactionOptions?.callOnBlock?.stringValue
   var request = JSONRPCrequest ()

   request.method = method
   let from = transactionOptions?.from
   quard var txParams = transaction.encodeAsDictionary(from: from) else { return nil }
   if method == .estimateGas || transactionOptions?.gasLimit == nil {
        txParams.gas = nil
   }
   var params = [txParams] as [Encodable]
   if method.requiredNumOfParameters == 2&& onBlock != nil {
      params.append( onBlock as Encodable )
   }
   let pars = JSONRPCparams(params: params)
   request.params = pars
   if !request.isValid { return nil }
   return request
}

Maybe there is a problem with this method, too.

Kristenlike1234 avatar Jul 01 '22 04:07 Kristenlike1234

@Kristenlike1234 Thanks for reporting! Will try to resolve that today. Will update you on my findings.

JeneaVranceanu avatar Jul 08 '22 08:07 JeneaVranceanu

@Kristenlike1234 Something that I didn't take into account right away is that TransactionOptions is a struct and since struct in Swift is a value type this action will create absolutely different variable:

var options = writeTransaction.transactionOptions

Modifying options won't change anything in writeTransaction.transactionOptions.

What you should do instead is change transactionOptions directly:

writeTransaction.transactionOptions.nonce = .latest
writeTransaction.transactionOptions.callOnBlock = .latest
writeTransaction.transactionOptions.type = .eip1559
writeTransaction.transactionOptions.maxFeePerGas = .manual(maxFee)
writeTransaction.transactionOptions.maxPriorityFeePerGas = .manual(maxTip)

If you prefer the way you do it with a new options variable then pass it into send or sendPromise functions like this:

var options = writeTransaction.transactionOptions
options.nonce = .latest
options.callOnBlock = .latest
options.type = .eip1559
options.maxFeePerGas = .manual(maxFee)
options.maxPriorityFeePerGas = .manual(maxTip)

writeTransaction.send(password: ..., transactionOptions: options)
// or
writeTransaction.sendPromise(password: ..., transactionOptions: options)

JeneaVranceanu avatar Jul 08 '22 08:07 JeneaVranceanu

Investigating further the following line from static func createRequest

guard var txParams = transaction.encodeAsDictionary(from: from) else { return nil }

will actually encode EIP1559Envelope if the TransactionType you used in TransactionOptions was .eip1559.

In Web3+MutatingTransaction.swift on the line 204 you can find your options being applied to the transaction that is being assembled and these options make its way into the EIP1559Envelope which is one of the implementations of AbstractEnvelope

// line 204
assembledTransaction.applyOptions(finalOptions)

JeneaVranceanu avatar Jul 08 '22 09:07 JeneaVranceanu

Here is even more proof that your transaction options will actually get into JSON RPC request's parameters. I've set maxFeePerGas to .manual(1) and maxPriorityFeePerGas to .manual(2) which are 0x1 and 0x2 in HEX respectively.

▿ TransactionParameters
  ▿ type : Optional<String>
    - some : "0x2"
  ▿ chainID : Optional<String>
    - some : "0x181d7807ea5"
  ▿ data : Optional<String>
    - some : "0xd93c982100000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000011536f6d6520696e70757420737472696e67000000000000000000000000000000"
  ▿ from : Optional<String>
    - some : "0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae"
  ▿ gas : Optional<String>
    - some : "0xf1140"
  - gasPrice : nil
  ▿ maxFeePerGas : Optional<String>
    - some : "0x1"
  ▿ maxPriorityFeePerGas : Optional<String>
    - some : "0x2"
  ▿ accessList : Optional<Array<AccessListEntry>>
    - some : 0 elements
  ▿ to : Optional<String>
    - some : "0xbb9bc244d798123fde783fcc1c72d3bb8c189413"
  ▿ value : Optional<String>
    - some : "0x759"
Screenshot 2022-07-08 at 12 33 37

JeneaVranceanu avatar Jul 08 '22 09:07 JeneaVranceanu

@Kristenlike1234 Try passing back your options into send or sendPromise and ping me with the result, please. If that won't help we should look further. For now, I'll switch to a different task.

JeneaVranceanu avatar Jul 08 '22 09:07 JeneaVranceanu