Refactor transaction-confirmation to use new transaction lifetimes

[mcintyre94]

This PR refactors transaction-confirmation to take NewTransaction & {lifetimeConstraint: ...} as input. This removes the dependency on the old transaction model.

• The new Transaction blockhash lifetime has lastValidBlockHeight added
• The new Transaction durable nonce lifetime has nonceAccountAddress added. Previously this was pulled from the transaction's instructions, but we no longer have access to decompiled instructions when we have a signed Transaction object.

Note: I'm leaning away from providing a helper to assert on the lifetime of a NewTransaction if you don't know it, at least for now. It would require decompiling almost all of the transaction (up to its first instruction), and potentially fetching address lookup tables (if they're used for any of the accounts in the advance nonce instruction). At that point I don't think it provides much value over our existing functions to decompile (using either known address table lookups or an rpc). We can always add this later if we find we need to, it doesn't block anything.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3b2ee65

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mcintyre94]

• Refactor send-transaction to use NewTransaction #2487 : 2 dependent PRs (#2488 , #2492 )
• add TransactionWithXLifetime types #2486
• Refactor transaction-confirmation to use new transaction lifetimes #2485 👈
• Maintain transaction lifetime on transactions #2484
• Attach a typed lifetime constraint field when we compile a transaction #2481
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mcintyre94 and the rest of your teammates on Graphite

[lorisleiva]

Could you remind me why we need the lifetimeConstraint to be on the NewTransaction type at all? Like where exactly is it used?

My other related question is: does this information already lives inside the compiled and encoded messageBytes property?

If so — and if the lifeConstraint is only used, say, when confirming a transaction — then, would it not make sense to decompile that information once there, instead of dragging these types everywhere with us?

[mcintyre94]

I think it's only used for transaction confirmation, but I haven't exhausively worked through everything using transactions yet so I'm not 100% sure if there are any other places. I think any other use cases will be much smaller though.

It lives in messageBytes - sort of. It's a durable nonce transaction if the first instruction is a particular durable nonce instruction. Our check for that also includes a lookup for the accounts, so if you wanted to do it in full then you'd need to combine it with either known address lookup tables or an RPC to resolve them. The accounts in that instruction could be in address lookup tables.

So we could take a NewTransaction and add a lifetime to it when we need it. But I think probably one of the most common use cases in web3js is that you create a transaction (at some point adding a lifetime), send and confirm it. We'd be requiring you to add this decompile (or use a helper that does it for you) on that path, even though you had the transaction message with a known lifetime. It's better performance and better DX.

I think in practice most people would probably just cast to whatever lifetime they were using instead of using a helper that requires address lookup tables or an RPC and not care that much, so the DX is the bigger point I think.

I also think the downside here is limited because there's just not much you need to/can do with a Transaction.

[mcintyre94]

This PR has ended up pulling in most of #2487 because it broke send-transaction

[lorisleiva]

Gotcha, it sounds like — whilst technically the information is there — the process required to get 100% of the compiled information is not simple enough to justify decompiling on-demand when we need access to that information. I'm happy with that.

I just wanted to make sure we didn't end up with another version of our "we need to keep our data in sync with the compiled bytes" problem — which is the primary reason for this entire refactoring haha.

[steveluscher]

Maybe Readonly<{...}> all of these?

[mcintyre94]

Good idea! I'll do this in an update to #2486 to avoid rebase conflicts there

[steveluscher]

[github-actions]

🎉 This PR is included in version 1.91.7 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.