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

add exception if we try to get chainId from a tx wo chainId (v = 27 o… #1210

Merged

Conversation

letypequividelespoubelles
Copy link
Collaborator

@letypequividelespoubelles letypequividelespoubelles commented Sep 17, 2024

Search for the chainId of the network in all tx of the conflation (instead of previously just the last tx). Will fail if all tx are wo chainId (will be fixed by hyperledger/besu#7630).

Add exception when trying to retrieve chainId of an unprotected tx (v == 27 or 28)

@letypequividelespoubelles letypequividelespoubelles linked an issue Sep 17, 2024 that may be closed by this pull request
@letypequividelespoubelles letypequividelespoubelles added the bug Something isn't working label Sep 17, 2024
Signed-off-by: Francois Bojarski <[email protected]>
@OlivierBBB OlivierBBB self-requested a review September 18, 2024 18:19
@@ -593,7 +593,7 @@ private void handlePhaseBeta(RlpTxnColumnsValue traceValue, Transaction tx, Trac
checkArgument(bigIntegerToBytes(V).size() <= 8, "V is longer than 8 bytes");
final boolean betaIsZero =
V.equals(BigInteger.valueOf(27))
|| V.equals(BigInteger.valueOf(28)); // beta = ChainId = 0 iff (V == 27 or V == 28)
|| V.equals(BigInteger.valueOf(28)); // beta = 0 iff (V == 27 or V == 28)
Copy link
Collaborator

Choose a reason for hiding this comment

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

My understanding is that if v $\in$ { 27, 28 } then we cannot draw any conclusions as to the value of $\beta$

Copy link
Collaborator

Choose a reason for hiding this comment

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

These transactions don't have replay protection and so can be replayed on any chain (that allows for such transactions.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we have a beta, but then beta is NOT the value of the chainId ...

final long V = tx.getV().longValueExact();
if (V == 27 || V == 28) {
return 0;
throw new IllegalArgumentException("ChainId not specified in transaction");
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purpose of throwing an exception ? To print a message ? I see that we catch it and resume execution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If there is any exception that would be problematic it's if some transaction defines a chainId which does not coincide with that of Linea mainnet / testnet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we call this functiun in phaseBeta of RLP_TXN and here we'll be happy to catch exception if needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's if some transaction defines a chainId which does not coincide with that of Linea mainnet / testnet.

Those transactions are supposed to be blocked by the sequencer.

final long V = tx.getV().longValueExact();
if (V == 27 || V == 28) {
return 0;
throw new IllegalArgumentException("ChainId not specified in transaction");
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there is any exception that would be problematic it's if some transaction defines a chainId which does not coincide with that of Linea mainnet / testnet.

@letypequividelespoubelles letypequividelespoubelles merged commit 20bb82c into arith-dev Sep 20, 2024
5 checks passed
@letypequividelespoubelles letypequividelespoubelles deleted the 1209-get-the-chainid-for-blockdata branch September 20, 2024 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get the chainId for BlockData
3 participants