Added new api to support conditional transactions (EIP-4337) #700
Conversation
| // Get the last block | ||
| block, err := b.blockByHash(ctx, b.pendingBlock.ParentHash()) | ||
| if err != nil { | ||
| return fmt.Errorf("could not fetch parent") |
There was a problem hiding this comment.
Could you use errors as predefined values? Also, I believe it's useful to add come context to each error and use errors.New() for defining new errors and fmt.Errorf("%w %v - any placeholders", errMyError, var1, var2...) providing additional context to an error.
There was a problem hiding this comment.
Right, this can be done.
This function is the replica of the SendTransaction function defined just above it. It is not used anywhere, and I have just kept it here because it was included in the interface.
| } | ||
| // Include tx in chain | ||
| blocks, _ := core.GenerateChain(b.config, block, ethash.NewFaker(), b.database, 1, func(number int, block *core.BlockGen) { | ||
| for _, tx := range b.pendingBlock.Transactions() { |
There was a problem hiding this comment.
could we remove variable shadowing?
There was a problem hiding this comment.
could pending block be changed during the for loop? is it thread safe to perform such loop?
There was a problem hiding this comment.
I think this function is used only for testing purposes, so I think this is fine. What do you think?
| stateDB, _ := b.blockchain.State() | ||
|
|
||
| b.pendingBlock = blocks[0] | ||
| b.pendingState, _ = state.New(b.pendingBlock.Root(), stateDB.Database(), nil) |
There was a problem hiding this comment.
could we have an error here?
There was a problem hiding this comment.
Yes, I will add an error check here. Thanks!
ethclient/ethclient_test.go
Outdated
| @@ -569,6 +569,8 @@ func testCallContract(t *testing.T, client *rpc.Client) { | |||
| func testAtFunctions(t *testing.T, client *rpc.Client) { | |||
| ec := NewClient(client) | |||
|
|
|||
| sendTransactionConditional(ec) | |||
There was a problem hiding this comment.
should we check the error here?
There was a problem hiding this comment.
Yes, we should. Will add the check. Thanks!
pratikspatil024
force-pushed
the
pos-1088
branch
2 times, most recently
from
0bcedcd to
503f3e5
Compare
Codecov ReportBase: 56.55% // Head: 56.59% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## aa-4337 #700 +/- ##
===========================================
+ Coverage 56.55% 56.59% +0.04%
===========================================
Files 610 610
Lines 71128 71146 +18
===========================================
+ Hits 40226 40267 +41
+ Misses 27448 27431 -17
+ Partials 3454 3448 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
internal/ethapi/api.go
Outdated
| if knownAccounts["knownAccounts"] == nil { | ||
| // should we allow this? | ||
| // throwing an error, for now | ||
| return common.Hash{}, errors.New("knownAccounts is empty") | ||
| } |
There was a problem hiding this comment.
I don't think we should assume this to be always non-empty. Bundlers should be fully responsible for this field.
There was a problem hiding this comment.
Okay, that makes sense.
pratikspatil024
force-pushed
the
pos-1088
branch
2 times, most recently
from
b4e84bb to
ca177db
Compare
pratikspatil024
force-pushed
the
pos-1088
branch
from
ca177db to
1675786
Compare
…port EIP-4337 Bundled Transactions (#945) * added new api to support conditional transactions (EIP-4337) (#700) * Refactored the code and updated the miner to check for the validity of options (#793) * refactored the code and updated the miner to check for the validity of options * added new errors -32003 and -32005 * added unit tests * addressed comments * Aa 4337 update generics (#799) * poc * minor bug fix * use common.Hash * updated UnmarshalJSON function (reference - tynes) * fix * done * linters * with test * undo some unintentional changes --------- Co-authored-by: Pratik Patil <pratikspatil024@gmail.com> * handelling the block range and timestamp range, also made timestamp a pointer --------- Co-authored-by: Evgeny Danilenko <6655321@bk.ru> * Added filtering of conditional transactions in txpool (#920) * added filtering of conditional transactions in txpool * minor fix in ValidateKnownAccounts * bug fix * Supporting nil knownAccounts * lints * bundled transactions are not announced/broadcasted to the peers * fixed after upstream merge * few fixes * sentry reject conditional transaction * Changed the namespace of conditional transaction API from `eth` to `bor` (#985) * added conditional transaction to bor namespace * test comit * test comit * added conditional transaction * namespapce changed to bor * cleanup * cleanup * addressed comments * reverted changes in ValidateKnownAccounts * addressed comments and removed unwanted code * addressed comments * bug fix * lint * removed licence from core/types/transaction_conditional_test.go --------- Co-authored-by: Evgeny Danilenko <6655321@bk.ru>
Added a new API to support conditional transactions.
Here is an example request: