Skip to content

feat: allow passing AztecNode to SentTx#12400

Closed
olehmisar wants to merge 2 commits intoAztecProtocol:masterfrom
olehmisar:om/sent-tx-with-node
Closed

feat: allow passing AztecNode to SentTx#12400
olehmisar wants to merge 2 commits intoAztecProtocol:masterfrom
olehmisar:om/sent-tx-with-node

Conversation

@olehmisar
Copy link
Contributor

resolves #11469

SentTx uses only public methods, so AztecNode also works when it's passed to SentTx. This is type-only change to not require users type cast AztecNode -> PXE.

There is also DeploySentTx and DeployAccountSentTx but I didn't change their signatures because not sure how useful it is.

@Maddiaa0 Maddiaa0 added the ci-external-once Run CI on an external PR, but only once. label Mar 3, 2025
@AztecBot AztecBot removed the ci-external-once Run CI on an external PR, but only once. label Mar 3, 2025
@Maddiaa0
Copy link
Member

Maddiaa0 commented Mar 3, 2025

ah nice stuff, if theres anywhere convenient to unit test this it would be great to add one to protect against regressions

@olehmisar
Copy link
Contributor Author

olehmisar commented Mar 3, 2025

@Maddiaa0 can't really do that as ./bootstrap.sh fast on github codespaces stopped working after 0.76.4. It gets indefinitely stuck on

noir-contracts  Executing: ./noir-contracts/bootstrap.sh fast (http://ci.aztec-labs.com/f0cac7fed120e1e4)

@Maddiaa0
Copy link
Member

Maddiaa0 commented Mar 3, 2025

@Maddiaa0 can't really do that as ./bootstrap.sh fast on github codespaces stopped working after 0.76.4. It gets indefinitely stuck on

noir-contracts  Executing: ./noir-contracts/bootstrap.sh fast (http://ci.aztec-labs.com/f0cac7fed120e1e4)

Ah i see, I assume the github codespaces machines have less than. 64GB of RAM, to fix run bootstrap with MEMSUSPEND=16GB, it defaults to 64GB

@olehmisar
Copy link
Contributor Author

@Maddiaa0 yeah, that helped. What do you want to unit test though? It’s only a type definition change

@Maddiaa0 Maddiaa0 requested a review from Thunkar March 3, 2025 15:56
ludamad added a commit that referenced this pull request Mar 3, 2025
…uspend (#12419)

## Overview

Fixes hanging in devboxes


see:
#12400 (comment)

---------

Co-authored-by: ludamad <domuradical@gmail.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
@Thunkar
Copy link
Contributor

Thunkar commented Mar 4, 2025

Hi @olehmisar, I redid this with some tests and a slight refactor to avoid confusing naming. Feel free to comment here

I don't love how it's implemented, though. All of these interactions are gonna be refactored, but I agree restricting interfaces for the sake of it is not good

@Thunkar Thunkar closed this Mar 4, 2025
@Maddiaa0
Copy link
Member

Maddiaa0 commented Mar 4, 2025

@Maddiaa0 yeah, that helped. What do you want to unit test though? It’s only a type definition change

Always need unit tests

@olehmisar olehmisar deleted the om/sent-tx-with-node branch March 12, 2025 11:43
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.

SentTx should accept AztecNode instead of PXE

4 participants