Skip to content

Commit

Permalink
Tx: Execute on deprecation notes (#1742)
Browse files Browse the repository at this point in the history
* Tx: Remove baseTransaction.transactionType

* tx: remove get senderR()

* tx: remove test for senderR()

* tx: remove senderS()

* tx: remove test for senderS()

* tx: remove yParity / replace with v

* tx: remove test for yParity()

* Tx: remove fromRlpSerializedTx()

* Tx: remove test for fromRlpSerializedTx()

* Tx: remove _unsignedTxImplementsEIP155

* Tx: Remove _signedTxImplementsEIP155()

* Tx: Remove TransactionFactory.getTransactionClass

* Tx: Remove test for .getTransactionClass()
  • Loading branch information
ScottyPoi authored and holgerd77 committed Apr 5, 2022
1 parent 52bed1b commit b45b146
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 189 deletions.
9 changes: 0 additions & 9 deletions packages/tx/src/baseTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,6 @@ export abstract class BaseTransaction<TransactionObject> {
this._validateCannotExceedMaxInteger({ nonce: this.nonce }, 64, true)
}

/**
* Alias for {@link BaseTransaction.type}
*
* @deprecated Use `type` instead
*/
get transactionType(): number {
return this.type
}

/**
* Returns the transaction type.
*
Expand Down
40 changes: 0 additions & 40 deletions packages/tx/src/eip1559Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,33 +48,6 @@ export default class FeeMarketEIP1559Transaction extends BaseTransaction<FeeMark
*/
protected DEFAULT_HARDFORK = 'london'

/**
* EIP-2930 alias for `r`
*
* @deprecated use `r` instead
*/
get senderR() {
return this.r
}

/**
* EIP-2930 alias for `s`
*
* @deprecated use `s` instead
*/
get senderS() {
return this.s
}

/**
* EIP-2930 alias for `v`
*
* @deprecated use `v` instead
*/
get yParity() {
return this.v
}

/**
* Instantiate a transaction from a data dictionary.
*
Expand Down Expand Up @@ -113,19 +86,6 @@ export default class FeeMarketEIP1559Transaction extends BaseTransaction<FeeMark
return FeeMarketEIP1559Transaction.fromValuesArray(values as any, opts)
}

/**
* Instantiate a transaction from the serialized tx.
* (alias of {@link FeeMarketEIP1559Transaction.fromSerializedTx})
*
* Note: This means that the Buffer should start with 0x01.
*
* @deprecated this constructor alias is deprecated and will be removed
* in favor of the {@link FeeMarketEIP1559Transaction.fromSerializedTx} constructor
*/
public static fromRlpSerializedTx(serialized: Buffer, opts: TxOptions = {}) {
return FeeMarketEIP1559Transaction.fromSerializedTx(serialized, opts)
}

/**
* Create a transaction from a values array.
*
Expand Down
44 changes: 2 additions & 42 deletions packages/tx/src/eip2930Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,33 +48,6 @@ export default class AccessListEIP2930Transaction extends BaseTransaction<Access
*/
protected DEFAULT_HARDFORK = 'berlin'

/**
* EIP-2930 alias for `r`
*
* @deprecated use `r` instead
*/
get senderR() {
return this.r
}

/**
* EIP-2930 alias for `s`
*
* @deprecated use `s` instead
*/
get senderS() {
return this.s
}

/**
* EIP-2930 alias for `v`
*
* @deprecated use `v` instead
*/
get yParity() {
return this.v
}

/**
* Instantiate a transaction from a data dictionary.
*
Expand Down Expand Up @@ -113,19 +86,6 @@ export default class AccessListEIP2930Transaction extends BaseTransaction<Access
return AccessListEIP2930Transaction.fromValuesArray(values as any, opts)
}

/**
* Instantiate a transaction from the serialized tx.
* (alias of {@link AccessListEIP2930Transaction.fromSerializedTx})
*
* Note: This means that the Buffer should start with 0x01.
*
* @deprecated this constructor alias is deprecated and will be removed
* in favor of the {@link AccessListEIP2930Transaction.fromSerializedTx} constructor
*/
public static fromRlpSerializedTx(serialized: Buffer, opts: TxOptions = {}) {
return AccessListEIP2930Transaction.fromSerializedTx(serialized, opts)
}

/**
* Create a transaction from a values array.
*
Expand Down Expand Up @@ -365,11 +325,11 @@ export default class AccessListEIP2930Transaction extends BaseTransaction<Access
throw new Error(msg)
}

const { yParity, r, s } = this
const { v, r, s } = this
try {
return ecrecover(
msgHash,
yParity!.addn(27), // Recover the 27 which was stripped from ecsign
v!.addn(27), // Recover the 27 which was stripped from ecsign
bnToUnpaddedBuffer(r!),
bnToUnpaddedBuffer(s!)
)
Expand Down
40 changes: 0 additions & 40 deletions packages/tx/src/legacyTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,6 @@ export default class Transaction extends BaseTransaction<Transaction> {
return this.fromValuesArray(values, opts)
}

/**
* Instantiate a transaction from the serialized tx.
* (alias of {@link Transaction.fromSerializedTx})
*
* @deprecated this constructor alias is deprecated and will be removed
* in favor of the {@link Transaction.fromSerializedTx} constructor
*/
public static fromRlpSerializedTx(serialized: Buffer, opts: TxOptions = {}) {
return Transaction.fromSerializedTx(serialized, opts)
}

/**
* Create a transaction from a values array.
*
Expand Down Expand Up @@ -413,35 +402,6 @@ export default class Transaction extends BaseTransaction<Transaction> {
return this._getCommon(common, chainIdBN)
}

/**
* @deprecated if you have called this internal method please use `tx.supports(Capabilities.EIP155ReplayProtection)` instead
*/
private _unsignedTxImplementsEIP155() {
return this.common.gteHardfork('spuriousDragon')
}

/**
* @deprecated if you have called this internal method please use `tx.supports(Capabilities.EIP155ReplayProtection)` instead
*/
private _signedTxImplementsEIP155() {
if (!this.isSigned()) {
const msg = this._errorMsg('This transaction is not signed')
throw new Error(msg)
}
const onEIP155BlockOrLater = this.common.gteHardfork('spuriousDragon')

// EIP155 spec:
// If block.number >= 2,675,000 and v = CHAIN_ID * 2 + 35 or v = CHAIN_ID * 2 + 36, then when computing the hash of a transaction for purposes of signing or recovering, instead of hashing only the first six elements (i.e. nonce, gasprice, startgas, to, value, data), hash nine elements, with v replaced by CHAIN_ID, r = 0 and s = 0.
const v = this.v!

const chainIdDoubled = this.common.chainId().muln(2)

const vAndChainIdMeetEIP155Conditions =
v.eq(chainIdDoubled.addn(35)) || v.eq(chainIdDoubled.addn(36))

return vAndChainIdMeetEIP155Conditions && onEIP155BlockOrLater
}

/**
* Return a compact error string representation of the object
*/
Expand Down
25 changes: 0 additions & 25 deletions packages/tx/src/transactionFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
FeeMarketEIP1559TxData,
} from './types'
import { Transaction, AccessListEIP2930Transaction, FeeMarketEIP1559Transaction } from '.'
import Common from '@ethereumjs/common'

export default class TransactionFactory {
// It is not possible to instantiate a TransactionFactory object.
Expand Down Expand Up @@ -90,28 +89,4 @@ export default class TransactionFactory {
throw new Error('Cannot decode transaction: unknown type input')
}
}

/**
* This helper method allows one to retrieve the class which matches the transactionID
* If transactionID is undefined, returns the legacy transaction class.
* @deprecated - This method is deprecated and will be removed on the next major release
* @param transactionID
* @param _common - This option is not used
*/
public static getTransactionClass(transactionID: number = 0, _common?: Common) {
const legacyTxn = transactionID == 0 || (transactionID >= 0x80 && transactionID <= 0xff)

if (legacyTxn) {
return Transaction
}

switch (transactionID) {
case 1:
return AccessListEIP2930Transaction
case 2:
return FeeMarketEIP1559Transaction
default:
throw new Error(`TypedTransaction with ID ${transactionID} unknown`)
}
}
}
7 changes: 0 additions & 7 deletions packages/tx/test/base.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,6 @@ tape('[BaseTransaction]', function (t) {

st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)

tx = txType.class.fromRlpSerializedTx(rlpData, { common })
st.equal(
tx.type,
txType.type,
`${txType.name}: fromRlpSerializedTx() (deprecated) -> should initialize correctly`
)

tx = txType.class.fromSerializedTx(rlpData, { common, freeze: false })
st.ok(
!Object.isFrozen(tx),
Expand Down
21 changes: 0 additions & 21 deletions packages/tx/test/transactionFactory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,25 +147,4 @@ tape('[TransactionFactory]: Basic functions', function (t) {

st.end()
})

t.test('getTransactionClass() -> success cases', function (st) {
const legacyTx = TransactionFactory.getTransactionClass()
st.equals(legacyTx!.name, Transaction.name)

for (const txType of txTypes) {
if (!txType.eip2718) {
const tx = TransactionFactory.getTransactionClass(txType.type)
st.equals(tx.name, txType.class.name)
}
}
st.end()
})

t.test('getTransactionClass() -> error cases', function (st) {
st.throws(() => {
TransactionFactory.getTransactionClass(3)
}, 'should throw when getting an invalid transaction type')

st.end()
})
})
6 changes: 1 addition & 5 deletions packages/tx/test/typedTxsAndEIP2930.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ tape(
st.end()
})

t.test('sign() / senderS(), senderR(), yParity()', function (t) {
t.test('sign()', function (t) {
for (const txType of txTypes) {
let tx = txType.class.fromTxData(
{
Expand All @@ -281,10 +281,6 @@ tape(
tx = txType.class.fromTxData({}, { common })
signed = tx.sign(pKey)

t.deepEqual(signed.senderR, signed.r, `should provide senderR() alias (${txType.name})`)
t.deepEqual(signed.senderS, signed.s, `should provide senderS() alias (${txType.name})`)
t.deepEqual(signed.yParity, signed.v, `should provide yParity() alias (${txType.name})`)

t.deepEqual(
tx.accessList,
[],
Expand Down

0 comments on commit b45b146

Please sign in to comment.