Skip to content

Conversation

@tomaka
Copy link
Contributor

@tomaka tomaka commented Apr 3, 2024

Reverts #144

#146 is flawed. This shouldn't have been stabilized.

lexnv
lexnv previously approved these changes Apr 3, 2024
Copy link
Contributor

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

LGTM!

Just to double-check, we are reverting this because the imposed limit of having at least 4 subscriptions accepted per connection cannot be guaranteed once the transaction enters the pool.

Therefore, we might have situations where the transctionWatch is capped to 4 subscriptions although the same user has more than 4 transactions currently active in the pool.

jsdw
jsdw previously approved these changes Apr 3, 2024
@jsdw
Copy link
Collaborator

jsdw commented Apr 3, 2024

I resolved the weird conflict in #149 since I can't easily edit fork PRs :)

Will close this and merge that if that's all ok!

@jsdw jsdw changed the base branch from main to revert-transactionwatch-v1 April 3, 2024 16:09
@jsdw jsdw changed the base branch from revert-transactionwatch-v1 to main April 3, 2024 16:09
@jsdw jsdw dismissed stale reviews from lexnv and themself April 3, 2024 16:09

The base branch was changed.

@jsdw jsdw closed this Apr 3, 2024
@tomaka tomaka deleted the revert-144-lexnv/stabilize-tx-watch branch April 4, 2024 06:01
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.

4 participants