Skip to content

Add ArbitrumRetryTx type #12

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
Closed
wants to merge 14 commits into from
Closed

Add ArbitrumRetryTx type #12

wants to merge 14 commits into from

Conversation

edfelten
Copy link
Contributor

@edfelten edfelten commented Oct 28, 2021 •
edited
Loading

Add Arbitrum tx types needed for retryables.

@edfelten edfelten requested a review from PlasmaPower October 28, 2021 12:21
@PlasmaPower
Copy link
Contributor

I think we need a couple associated changes:

  • We should define a new tx type byte and serialization/deserialization. I'd search the code for for ArbitrumDepositTxType, make a new constant, and use it everywhere that one is used.
  • This new type won't be downcastable to an ArbitrumContractTx. This is important for e.g. the arbitrum signer, so we probably need to change that code (and maybe ctrl+f for anywhere ArbitrumContractTx is used).

PlasmaPower
PlasmaPower reviewed Oct 29, 2021
View reviewed changes
@PlasmaPower
Copy link
Contributor

re: serialization/deserialization, any transaction in a block needs to be (de)serialized, as geth needs to store the block. We also only need transaction types for things that actually make it into blocks and change state.

@edfelten edfelten requested a review from PlasmaPower November 24, 2021 13:47
PlasmaPower
PlasmaPower suggested changes Dec 14, 2021
View reviewed changes
core/types/transaction_signing.go
case ArbitrumRetryTxType:
return tx.inner.(*ArbitrumRetryTx).From, nil
case ArbitrumSubmitRetryableTxType:
return tx.inner.(*ArbitrumSubmitRetryableTx).From, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe these cases should happen in the ArbitrumSigner, which we always instantiate on Arbitrum chains

core/types/transaction.go
} else if tx.Type() == ArbitrumSubmitRetryableTxType {
h = tx.inner.(*ArbitrumSubmitRetryableTx).RequestId
} else if tx.Type() == ArbitrumRetryTxType {
h = tx.inner.(*ArbitrumRetryTx).RequestId
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem a bit dangerous to override if we could avoid it, but I'm guessing using the request id as the hash really simplifies things?

core/types/transaction.go
@@ -170,7 +172,7 @@ func (tx *Transaction) UnmarshalBinary(b []byte) error {
return nil
}
// It's an EIP2718 typed transaction envelope.
inner, err := tx.decodeTyped(b, false)
inner, err := tx.decodeTyped(b, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want arb parsing here, as this function is called for external input, e.g. sendRawTransaction. We only want to accept normal transaction types there.

core/types/arb_types.go
ChainId: new(big.Int),
RequestId: tx.RequestId,
DepositValue: tx.DepositValue,
GasPrice: tx.GasPrice,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe all the big ints here should be new(big.Int) like ChainId. As-is, I don't think it does a deep copy.

@edfelten
Copy link
Contributor Author

Closing this. It was superseded by other changes.

@edfelten edfelten closed this Dec 15, 2021
rauljordan pushed a commit to rauljordan/go-ethereum that referenced this pull request Oct 20, 2022
@Inphi
add --eip4844 devnet flag (OffchainLabs#12)
769f148 
Configures the genesis and network parameters of the protodanksharding devnet.
@tsahee tsahee deleted the add-retry-tx-type branch November 28, 2023 17:44
zfy0701 pushed a commit to sentioxyz/nitro-geth that referenced this pull request Mar 11, 2024
@lwedge99 @zfy0701
minor fix (OffchainLabs#12)
0441fa5
zfy0701 pushed a commit to sentioxyz/nitro-geth that referenced this pull request Mar 11, 2024
@lwedge99 @zfy0701
minor fix (OffchainLabs#12)
3053c43
zfy0701 added a commit to sentioxyz/nitro-geth that referenced this pull request Apr 24, 2024
@zfy0701
migrate sentio tracer to geth
4524af3 
migrate override and gas changes (OffchainLabs#1)

migrate memory compression (OffchainLabs#2)

add more information for root trace (OffchainLabs#3)

correct handle call with value (OffchainLabs#4)

* correct handle call with value

* set transfer value to zero if can't transfer

Add Mapping keys to post account (OffchainLabs#5)

fix when tracer failed before start (OffchainLabs#6)

unlimited gas for debug_traceCallMany (OffchainLabs#7)

support multiple txs in tracecallmany

rpc client should keep result while return error

be able to return partial results

migrate tracer changes (OffchainLabs#8)

export meq field (OffchainLabs#9)

ignore init code size limit (OffchainLabs#10)

code address field (OffchainLabs#11)

minor fix (OffchainLabs#12)

fix type compat (OffchainLabs#13)

fix compability with arb

fix storagerangeat

emit output for revert (OffchainLabs#14)
zfy0701 added a commit to sentioxyz/nitro-geth that referenced this pull request Jun 23, 2024
@zfy0701
migrate sentio tracer to geth
a029736 
migrate override and gas changes (OffchainLabs#1)

migrate memory compression (OffchainLabs#2)

add more information for root trace (OffchainLabs#3)

correct handle call with value (OffchainLabs#4)

* correct handle call with value

* set transfer value to zero if can't transfer

Add Mapping keys to post account (OffchainLabs#5)

fix when tracer failed before start (OffchainLabs#6)

unlimited gas for debug_traceCallMany (OffchainLabs#7)

support multiple txs in tracecallmany

rpc client should keep result while return error

be able to return partial results

migrate tracer changes (OffchainLabs#8)

export meq field (OffchainLabs#9)

ignore init code size limit (OffchainLabs#10)

code address field (OffchainLabs#11)

minor fix (OffchainLabs#12)

fix type compat (OffchainLabs#13)

fix compability with arb

fix storagerangeat

emit output for revert (OffchainLabs#14)
zfy0701 added a commit to sentioxyz/nitro-geth that referenced this pull request Jun 23, 2024
@zfy0701
migrate sentio tracer to geth
9d94bc9 
migrate override and gas changes (OffchainLabs#1)

migrate memory compression (OffchainLabs#2)

add more information for root trace (OffchainLabs#3)

correct handle call with value (OffchainLabs#4)

* correct handle call with value

* set transfer value to zero if can't transfer

Add Mapping keys to post account (OffchainLabs#5)

fix when tracer failed before start (OffchainLabs#6)

unlimited gas for debug_traceCallMany (OffchainLabs#7)

support multiple txs in tracecallmany

rpc client should keep result while return error

be able to return partial results

migrate tracer changes (OffchainLabs#8)

export meq field (OffchainLabs#9)

ignore init code size limit (OffchainLabs#10)

code address field (OffchainLabs#11)

minor fix (OffchainLabs#12)

fix type compat (OffchainLabs#13)

fix compability with arb

fix storagerangeat

emit output for revert (OffchainLabs#14)
zfy0701 added a commit to sentioxyz/nitro-geth that referenced this pull request Jul 2, 2024
@zfy0701
migrate sentio tracer to geth
dcf5eea 
migrate override and gas changes (OffchainLabs#1)

migrate memory compression (OffchainLabs#2)

add more information for root trace (OffchainLabs#3)

correct handle call with value (OffchainLabs#4)

* correct handle call with value

* set transfer value to zero if can't transfer

Add Mapping keys to post account (OffchainLabs#5)

fix when tracer failed before start (OffchainLabs#6)

unlimited gas for debug_traceCallMany (OffchainLabs#7)

support multiple txs in tracecallmany

rpc client should keep result while return error

be able to return partial results

migrate tracer changes (OffchainLabs#8)

export meq field (OffchainLabs#9)

ignore init code size limit (OffchainLabs#10)

code address field (OffchainLabs#11)

minor fix (OffchainLabs#12)

fix type compat (OffchainLabs#13)

fix compability with arb

fix storagerangeat

emit output for revert (OffchainLabs#14)
zfy0701 added a commit to sentioxyz/nitro-geth that referenced this pull request Sep 2, 2024
@zfy0701
migrate sentio tracer to geth
3ab3d3c 
migrate override and gas changes (OffchainLabs#1)

migrate memory compression (OffchainLabs#2)

add more information for root trace (OffchainLabs#3)

correct handle call with value (OffchainLabs#4)

* correct handle call with value

* set transfer value to zero if can't transfer

Add Mapping keys to post account (OffchainLabs#5)

fix when tracer failed before start (OffchainLabs#6)

unlimited gas for debug_traceCallMany (OffchainLabs#7)

support multiple txs in tracecallmany

rpc client should keep result while return error

be able to return partial results

migrate tracer changes (OffchainLabs#8)

export meq field (OffchainLabs#9)

ignore init code size limit (OffchainLabs#10)

code address field (OffchainLabs#11)

minor fix (OffchainLabs#12)

fix type compat (OffchainLabs#13)

fix compability with arb

fix storagerangeat

emit output for revert (OffchainLabs#14)
zfy0701 added a commit to sentioxyz/nitro-geth that referenced this pull request Sep 26, 2024
@zfy0701
migrate sentio tracer to geth
4ccf823 
migrate override and gas changes (OffchainLabs#1)

migrate memory compression (OffchainLabs#2)

add more information for root trace (OffchainLabs#3)

correct handle call with value (OffchainLabs#4)

* correct handle call with value

* set transfer value to zero if can't transfer

Add Mapping keys to post account (OffchainLabs#5)

fix when tracer failed before start (OffchainLabs#6)

unlimited gas for debug_traceCallMany (OffchainLabs#7)

support multiple txs in tracecallmany

rpc client should keep result while return error

be able to return partial results

migrate tracer changes (OffchainLabs#8)

export meq field (OffchainLabs#9)

ignore init code size limit (OffchainLabs#10)

code address field (OffchainLabs#11)

minor fix (OffchainLabs#12)

fix type compat (OffchainLabs#13)

fix compability with arb

fix storagerangeat

emit output for revert (OffchainLabs#14)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PlasmaPower PlasmaPower PlasmaPower requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@edfelten @PlasmaPower