-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: 764 add new rpc endpoints metamask #765
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
==========================================
+ Coverage 18.21% 18.38% +0.17%
==========================================
Files 129 129
Lines 10076 10118 +42
Branches 302 306 +4
==========================================
+ Hits 1835 1860 +25
- Misses 8156 8173 +17
Partials 85 85 ☔ View full report in Codecov by Sentry. |
def get_block_by_number( | ||
transactions_processor: TransactionsProcessor, block_number: str, full_tx: bool | ||
) -> dict | None: | ||
try: | ||
block_number_int = int(block_number, 16) | ||
except ValueError: | ||
raise JSONRPCError(f"Invalid block number format: {block_number}") | ||
|
||
block_details = transactions_processor.get_transactions_for_block( | ||
block_number_int, include_full_tx=full_tx | ||
) | ||
|
||
if not block_details: | ||
raise JSONRPCError(f"Block not found for number: {block_number}") | ||
|
||
return block_details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the signature dict | None
? I don't see anywhere where it could return None
|
||
receipt = { | ||
"transactionHash": transaction_hash, | ||
"transactionIndex": hex(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because we just have one transaction per "block"
receipt = { | ||
"transactionHash": transaction_hash, | ||
"transactionIndex": hex(0), | ||
"blockHash": transaction_hash, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is a block the same as a transaction for us? a small explanation in a comment would be great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have a block table or block hash rn, so yes, at this moment I'm considering transaction = block
transaction_hash: str, | ||
) -> dict | None: | ||
|
||
transaction = transactions_processor.get_transaction_by_hash(transaction_hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could probably take advantage of the Transaction
type if we returned some dataclass instead of a dict. This way we wouldn't need to do so many transaction.get
below
else None | ||
), | ||
"logs": transaction.get("logs", []), | ||
"logsBloom": "0x" + "00" * 256, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will we fill this with something at some point?
) -> dict: | ||
transactions = ( | ||
self.session.query(Transactions) | ||
.filter(Transactions.nonce == block_number) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should explain somehow that block_number === nonce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed from nonce to timestamp here: 20b83f0
timestamp is the only unique key on transactions table
Quality Gate failedFailed conditions |
Fixes #764
What
is_valid_address
withis_address
for Ethereum address validation in theaccounts_manager
.get_block_by_number
andget_block_by_hash
, to the RPC endpoints.AccountSelect.vue
.useGenLayer
andaccounts.ts
to handle MetaMask account logic and wallet selection.transactions_processor.py
:global.d.ts
file to define theethereum
object for TypeScript support.Why
Testing Done
fetchMetaMaskAccount
method.Decisions Made
is_address
overis_valid_address
for Ethereum validation for improved consistency across the codebase.Checks
Reviewing Tips
transactions_processor.py
and the new MetaMask logic in the frontend.User-Facing Release Notes