update wait_for_tansaction api to support long poll

[fishronsage]

No description provided.

[davidiw]

thank you for your contribution!!!

From reading the spec here: https://fullnode.mainnet.aptoslabs.com/v1/spec#/operations/wait_transaction_by_hash

it seems like we should just use this as a drop in replacement for the existing code, so it should just be a small amount of changes.

I'm not sure we need to try to use both code paths at this point unless we found that there's a perceivable performance difference.

[davidiw]

Coming back to this within the next 24 hours.

[fishronsage]

(c3003a9 paste the previous review here for convenient)

[davidiw]

do I need to migrate my comments here?

[fishronsage]

do I need to migrate my comments here?

I think we can discuss here for new comments and leave old ones there since it's easy to track. 😄

[davidiw]

curious, after giving feedback in the previous commit, it doesn't look like this was updated. is that intended? @fishronsage

[fishronsage]

@davidiw No. I had replied your feedback and pinged you in c3003a9. Thought you might be too busy to response 😂

[davidiw]

lemme know what you think of the tweak and then we can land it.

[davidiw]

Why are the tests failing 🤔

[fishronsage]

I think it could be due to some settings of devent? I ran this on testnet and it passed. I also ran the ts sdk's example, got 404 too on devent.

[fishronsage]

Yeah, I regret that statement. If we get a 404, it likely means something else since we enabled stickiness. We could check to see that stickiness is enabled, but it is by default 🤷

(c3003a9#r146052694)

BTW, I got 404 on testnet. maybe 404 should be still handled? 🤷‍♂️

[fishronsage]

Oh, https://github.com/aptos-labs/aptos-python-sdk/actions/runs/10852701073/job/30120071280?pr=10 did get 404 on testnet...

[davidiw]

Okay, let's wait to land this until we figure out why the infra is inconsistent with our expectations. I'm following up with the maintainers of the fullnodes.