Skip to content

Conversation

Marchhill
Copy link
Contributor

@Marchhill Marchhill commented Jan 3, 2025

Changes

  • Implement EIP-7805
  • engine api changes
    • implement engine_getInclusionList
    • add inclusion lists to PayloadAttributes and newPayload parameters
  • validate that valid IL txs were included up to gas limit (see here)
  • use IL in block building

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

Comment on lines 33 to 34
TrieStore trieStore = TestTrieStoreFactory.Build(stateDb, LimboLogs.Instance);
_stateProvider = new WorldState(trieStore, new MemDb(), LimboLogs.Instance);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if needs update after: #8724

}

block.InclusionListTransactions = suggestedBlock.InclusionListTransactions;
return _inclusionListValidator.ValidateInclusionList(block, _blockTransactionsExecutor.IsTransactionInBlock);
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need IsTransactionInBlock in executor and can simply check against block.Transactions?

}
}

public void RecoverData(Transaction[] txs, IReleaseSpec releaseSpec, bool checkFirst)
Copy link
Member

Choose a reason for hiding this comment

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

why do we need different checkFirst mechanic?

/// <c>true</c> if the block's inclusion list is satisfied according to EIP-7805;
/// otherwise, <c>false</c>.
/// </returns>
bool ValidateInclusionList(Block block, Func<Transaction, bool> isTransactionInBlock);
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to pass isTransactionInBlock? You should be able to check against block.Transactions

Comment on lines +154 to +162
public void ForceRebuildPayload(string payloadId)
{
throw new NotImplementedException();
}

public BlockHeader? GetPayloadHeader(string payloadId)
{
throw new NotImplementedException();
}
Copy link
Member

Choose a reason for hiding this comment

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

maybe interface segregation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants