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

adds AddTransactions, AddBundle, AddBundles to the builder API #23

Merged
merged 6 commits into from
Mar 14, 2024

Conversation

jinmel
Copy link

@jinmel jinmel commented Mar 12, 2024

No description provided.

return builder.AddTransactions(txs)
}

func (s *SessionManager) AddBundle(sessionId string, bundle *api.Bundle) (*api.SimulateBundleResult, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we merge AddBundle with AddBundles? That way we can reduce the scope of the API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that we cannot do the same with AddTransactions because of the backward compatibility.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -39,8 +39,16 @@ func (nullSessionManager) AddTransaction(sessionId string, tx *types.Transaction
return &SimulateTransactionResult{Logs: []*SimulatedLog{}}, nil
}

func (nullSessionManager) AddBundle(sessionId string, bundle Bundle) error {
return nil
func (nullSessionManager) AddTransactions(sessionId string, txs types.Transactions) ([]*SimulateTransactionResult, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you call these functions in TestAPI test? It is to make sure that both server and client encode the same thing.

Copy link
Author

Choose a reason for hiding this comment

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

Did for AddTransactions too

@ferranbt ferranbt merged commit cebeb5d into flashbots:main Mar 14, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants