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

fix!: make encoding/decoding RLP lists non-ambiguous by returning empty List instead of null #65

Merged
merged 3 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ abstract class ReadWriteContractCall<C, S : PendingInclusion<*>, B : ReadWriteCo
}

if (gasPrice != null) {
if (accessList != null) {
if (accessList.isNotEmpty()) {
return TxAccessList(
to = to,
value = value ?: BigInteger.ZERO,
Expand All @@ -103,7 +103,7 @@ abstract class ReadWriteContractCall<C, S : PendingInclusion<*>, B : ReadWriteCo
gasPrice = gasPrice!!,
data = data,
chainId = chainId,
accessList = accessList!!,
accessList = accessList,
)
}

Expand Down Expand Up @@ -221,7 +221,7 @@ abstract class ReadContractCall<C, B : ReadContractCall<C, B>>(
call.nonce = value
}

var accessList: List<AccessList.Item>?
var accessList: List<AccessList.Item>
get() = call.accessList
@JvmSynthetic set(value) {
call.accessList = value
Expand Down Expand Up @@ -257,7 +257,7 @@ abstract class ReadContractCall<C, B : ReadContractCall<C, B>>(
return self
}

fun accessList(value: List<AccessList.Item>?): B {
fun accessList(value: List<AccessList.Item>): B {
call.accessList = value
return self
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ object AccessList {
override fun rlpDecode(rlp: RlpDecoder): Item? {
return rlp.decodeList {
val address = rlp.decode(Address) ?: return null
val storageKeys = rlp.decodeAsList(Hash) ?: return null
val storageKeys = rlp.decodeAsList(Hash)

Item(address, storageKeys)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class CallRequest {
@JvmSynthetic set
var data: Bytes? = null
@JvmSynthetic set
var accessList: List<AccessList.Item>? = null
var accessList: List<AccessList.Item> = emptyList()
@JvmSynthetic set
var chainId: Long = -1L
@JvmSynthetic set
Expand Down Expand Up @@ -129,9 +129,9 @@ private class CallRequestSerializer : JsonSerializer<CallRequest>() {
if (value.data != null) {
gen.writeStringField("input", value.data.toString())
}
if (value.accessList != null) {
if (value.accessList.isNotEmpty()) {
gen.writeArrayFieldStart("accessList")
for (item in value.accessList!!) {
for (item in value.accessList) {
// delegate to AccessList.Item serializer
gen.writeObject(item)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ data class RPCTransaction(
override val gasFeeCap: BigInteger,
override val gasTipCap: BigInteger,
override val data: Bytes?,
override val accessList: List<AccessList.Item>?,
override val accessList: List<AccessList.Item>,
override val chainId: Long,
override val type: TxType,
val v: Long,
Expand Down Expand Up @@ -68,7 +68,7 @@ private class RPCTransactionDeserializer : JsonDeserializer<RPCTransaction>() {
var gasTipCap: BigInteger? = null
var data: Bytes? = null
var type = -1L
var accessList: List<AccessList.Item>? = null
var accessList: List<AccessList.Item> = emptyList()
var chainId: Long = ChainId.NONE
var v = -1L
lateinit var r: BigInteger
Expand All @@ -95,7 +95,7 @@ private class RPCTransactionDeserializer : JsonDeserializer<RPCTransaction>() {
"input" -> data = p.readBytesEmptyAsNull()
"type" -> type = p.readHexLong()
"accessList" -> accessList = p.readListOf(AccessList.Item::class.java)
"chainId" -> chainId = p.readHexLong()
"chainId" -> p.ifNotNull { chainId = p.readHexLong() }
"v" -> v = p.readHexLong()
"r" -> r = p.readHexBigInteger()
"s" -> s = p.readHexBigInteger()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ interface Transaction {
val gasFeeCap: BigInteger
val data: Bytes?
val chainId: Long
val accessList: List<AccessList.Item>?
val accessList: List<AccessList.Item>
val type: TxType
val blobFeeCap: BigInteger?
val blobVersionedHashes: List<Hash>?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class TxAccessList(
override val gasPrice: BigInteger,
override val data: Bytes?,
override val chainId: Long,
override var accessList: List<AccessList.Item>?,
override var accessList: List<AccessList.Item>,
) : TransactionUnsigned {
init {
if (!ChainId.isValid(chainId)) {
Expand Down Expand Up @@ -67,7 +67,7 @@ class TxAccessList(
gasPrice: BigInteger = this.gasPrice,
data: Bytes? = this.data,
chainId: Long = this.chainId,
accessList: List<AccessList.Item>? = this.accessList,
accessList: List<AccessList.Item> = this.accessList,
): TxAccessList {
return TxAccessList(
to = to,
Expand Down Expand Up @@ -107,7 +107,7 @@ class TxAccessList(
result = 31 * result + gasPrice.hashCode()
result = 31 * result + (data?.hashCode() ?: 0)
result = 31 * result + chainId.hashCode()
result = 31 * result + (accessList?.hashCode() ?: 0)
result = 31 * result + accessList.hashCode()
return result
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class TxBlob(
override val gasTipCap: BigInteger,
override val data: Bytes?,
override val chainId: Long,
override var accessList: List<AccessList.Item>?,
override var accessList: List<AccessList.Item>,
override val blobFeeCap: BigInteger,
override val blobVersionedHashes: List<Hash>,
val sidecar: Sidecar? = null,
Expand All @@ -42,7 +42,7 @@ class TxBlob(
gasTipCap: BigInteger,
data: Bytes?,
chainId: Long,
accessList: List<AccessList.Item>?,
accessList: List<AccessList.Item>,
blobFeeCap: BigInteger,
sidecar: Sidecar,
) : this(
Expand Down Expand Up @@ -101,7 +101,7 @@ class TxBlob(
gasTipCap: BigInteger = this.gasTipCap,
data: Bytes? = this.data,
chainId: Long = this.chainId,
accessList: List<AccessList.Item>? = this.accessList,
accessList: List<AccessList.Item> = this.accessList,
blobFeeCap: BigInteger = this.blobFeeCap,
blobHashes: List<Hash> = this.blobVersionedHashes,
sidecar: Sidecar? = this.sidecar,
Expand Down Expand Up @@ -153,7 +153,7 @@ class TxBlob(
result = 31 * result + gasTipCap.hashCode()
result = 31 * result + (data?.hashCode() ?: 0)
result = 31 * result + chainId.hashCode()
result = 31 * result + (accessList?.hashCode() ?: 0)
result = 31 * result + (accessList.hashCode())
result = 31 * result + blobFeeCap.hashCode()
result = 31 * result + blobVersionedHashes.hashCode()
result = 31 * result + (sidecar?.hashCode() ?: 0)
Expand Down Expand Up @@ -202,11 +202,11 @@ class TxBlob(
const val COMMITMENT_LENGTH = 48
const val PROOF_LENGTH = 48

override fun rlpDecode(rlp: RlpDecoder): Sidecar? {
override fun rlpDecode(rlp: RlpDecoder): Sidecar {
return Sidecar(
blobs = rlp.decodeAsList(Bytes) ?: return null,
commitments = rlp.decodeAsList(Bytes) ?: return null,
proofs = rlp.decodeAsList(Bytes) ?: return null,
blobs = rlp.decodeAsList(Bytes),
commitments = rlp.decodeAsList(Bytes),
proofs = rlp.decodeAsList(Bytes),
)
}
}
Expand All @@ -228,7 +228,7 @@ class TxBlob(
data = rlp.decode(Bytes),
accessList = rlp.decodeAsList(AccessList.Item),
blobFeeCap = rlp.decodeBigIntegerElse(BigInteger.ZERO),
blobVersionedHashes = rlp.decodeAsList(Hash) ?: emptyList(),
blobVersionedHashes = rlp.decodeAsList(Hash),
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TxDynamicFee(
override val gasTipCap: BigInteger,
override val data: Bytes?,
override val chainId: Long,
override var accessList: List<AccessList.Item>?,
override var accessList: List<AccessList.Item>,
) : TransactionUnsigned {
init {
if (!ChainId.isValid(chainId)) {
Expand Down Expand Up @@ -70,7 +70,7 @@ class TxDynamicFee(
gasTipCap: BigInteger = this.gasTipCap,
data: Bytes? = this.data,
chainId: Long = this.chainId,
accessList: List<AccessList.Item>? = this.accessList,
accessList: List<AccessList.Item> = this.accessList,
): TxDynamicFee {
return TxDynamicFee(
to = to,
Expand Down Expand Up @@ -113,7 +113,7 @@ class TxDynamicFee(
result = 31 * result + gasTipCap.hashCode()
result = 31 * result + (data?.hashCode() ?: 0)
result = 31 * result + chainId.hashCode()
result = 31 * result + (accessList?.hashCode() ?: 0)
result = 31 * result + accessList.hashCode()
return result
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ data class TxLegacy(
override val gasFeeCap: BigInteger
get() = gasPrice

override val accessList: List<AccessList.Item>?
get() = null
override val accessList: List<AccessList.Item>
get() = emptyList()

override val type: TxType
get() = TxType.Legacy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ object TxAccessListFactory {
gas: Long = 0,
gasPrice: BigInteger = BigInteger.ZERO,
data: Bytes? = null,
accessList: List<AccessList.Item>? = null,
accessList: List<AccessList.Item> = emptyList(),
): TxAccessList {
return TxAccessList(to, value, nonce, gas, gasPrice, data, chainId, accessList)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ object TxDynamicFeeFactory {
gasFeeCap: BigInteger = BigInteger.ZERO,
gasTipCap: BigInteger = BigInteger.ZERO,
data: Bytes? = null,
accessList: List<AccessList.Item>? = null,
accessList: List<AccessList.Item> = emptyList(),
): TxDynamicFee {
return TxDynamicFee(to, value, nonce, gas, gasFeeCap, gasTipCap, data, chainId, accessList)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class TxpoolTest : FunSpec({
yParity = -1,
blobVersionedHashes = null,
blobFeeCap = null,
accessList = null,
accessList = emptyList(),
gasFeeCap = BigInteger("4010000000"),
gasTipCap = BigInteger("4010000000"),
),
Expand Down Expand Up @@ -111,7 +111,7 @@ class TxpoolTest : FunSpec({
yParity = -1,
blobVersionedHashes = null,
blobFeeCap = null,
accessList = null,
accessList = emptyList(),
gasFeeCap = BigInteger("7431800000"),
gasTipCap = BigInteger("7431800000"),
),
Expand Down Expand Up @@ -199,7 +199,7 @@ class TxpoolTest : FunSpec({
value = BigInteger("23000000000000000"),
type = TxType.Legacy,
chainId = 1L,
accessList = null,
accessList = emptyList(),
gasFeeCap = BigInteger("53739672778"),
gasTipCap = BigInteger("53739672778"),
v = 37,
Expand All @@ -225,7 +225,7 @@ class TxpoolTest : FunSpec({
value = BigInteger.ZERO,
type = TxType.Legacy,
chainId = 1L,
accessList = null,
accessList = emptyList(),
gasFeeCap = BigInteger("5500000000"),
gasTipCap = BigInteger("5500000000"),
v = 37,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.ethers.core.types.transactions

import io.ethers.core.Jackson
import io.ethers.core.types.AccessList
import io.ethers.core.types.Address
import io.ethers.core.types.Bytes
Expand All @@ -11,8 +12,12 @@ import io.ethers.core.types.transaction.TxBlob
import io.ethers.core.types.transaction.TxDynamicFee
import io.ethers.core.types.transaction.TxLegacy
import io.kotest.core.spec.style.FunSpec
import io.kotest.datatest.WithDataTestName
import io.kotest.datatest.withData
import io.kotest.matchers.shouldBe
import io.kotest.matchers.shouldNotBe
import io.kotest.matchers.string.shouldBeEqualIgnoringCase
import java.io.File

class TransactionSignedTest : FunSpec({
test("rlp encode/decode TxLegacy without chain ID") {
Expand Down Expand Up @@ -147,7 +152,7 @@ class TransactionSignedTest : FunSpec({
gasTipCap = "21000000000".toBigInteger(),
data = Bytes("0x1214abcdef12445980"),
chainId = 1L,
accessList = null,
accessList = emptyList(),
blobFeeCap = "21000000000".toBigInteger(),
blobVersionedHashes = listOf(Hash("0x010657f37554c781402a22917dee2f75def7ab966d7b770905398eba3c444014")),
)
Expand Down Expand Up @@ -178,7 +183,7 @@ class TransactionSignedTest : FunSpec({
gasTipCap = "21000000000".toBigInteger(),
data = Bytes("0x1214abcdef12445980"),
chainId = 1L,
accessList = null,
accessList = emptyList(),
blobFeeCap = "21000000000".toBigInteger(),
sidecar = TxBlob.Sidecar(
blobs = listOf(Bytes(ByteArray(TxBlob.Sidecar.BLOB_LENGTH))),
Expand All @@ -202,4 +207,46 @@ class TransactionSignedTest : FunSpec({
val decoded = TransactionSigned.rlpDecode(rlp)
decoded shouldBe signed
}
})

context("RLP roundtrip encode/decode test") {
val reader = Jackson.MAPPER.readerFor(RoundtripCase::class.java)
val rlpBatches = TransactionSignedTest::class.java.getResource("/testdata/txsigned")!!
.file
.let(::File)
.walkTopDown()
.filter { it.isFile && it.name.endsWith(".json") }
.map { it.name to reader.readValues<RoundtripCase>(it).readAll() }

rlpBatches.forEach { (dumpName, cases) ->
context(dumpName) {
withData(cases) {
val decoded = TransactionSigned.rlpDecode(it.rlp.value)

// it's enough to check that "hash" and "from" have expected values:
// - if any value was different from original tx, the "hash" will be different
// - if signature is different from original tx, the "from" will fail or will be different
decoded shouldNotBe null
decoded!!.hash shouldBe it.hash
decoded.from shouldBe it.from

val rlpEncoded = Bytes(decoded.toRlp())
rlpEncoded shouldBe it.rlp
}
}
}
}
}) {
data class RoundtripCase(val hash: Hash, val from: Address, val rlp: Bytes) : WithDataTestName {
override fun dataTestName() = hash.toString()
}
}

// Used to dump additional roundtrip test cases, need to run from providers module
/*fun main() {
val provider = Provider(HttpClient("RPC_URL"))
val block = provider.getBlockWithTransactions(19111477).sendAwait().unwrap()
val encoded = block.transactions.map { TransactionSignedTest.RoundtripCase(it.hash, it.from, Bytes(it.toSignedTransaction().toRlp())) }

// copy the result to resource folder
Jackson.MAPPER.writeValue(File("./transactions-${block.number}.json"), encoded)
}*/
Loading
Loading