Skip to content

Simplify remote data parsing - #178

Merged
radubahmata merged 3 commits into
masterfrom
fix/remote-data-parsing
Sep 25, 2025
Merged

Simplify remote data parsing#178
radubahmata merged 3 commits into
masterfrom
fix/remote-data-parsing

Conversation

@radubahmata

@radubahmata radubahmata commented Sep 24, 2025

Copy link
Copy Markdown
Contributor

This PR fixes an inconsistency when users omit repl.remote_data.api_url from their Clarinet configuration, while still specifying the other fields.

Clarinet defaults to the mainnet API URL when api_url is unset. Rendezvous treated this as if remote data was disabled and initialized an empty session without using remote data. This caused deployment failures because Rendezvous couldn't find requirements that Clarinet expected to be on mainnet

This updates Rendezvous so it matches Clarinet's behavior.

@radubahmata
radubahmata requested a review from a team as a code owner September 24, 2025 13:20
@radubahmata

Copy link
Copy Markdown
Contributor Author

@hugoclrd Can you please confirm this is always the default behavior? Does the API URL ever default to testnet (e.g., when requirement contract addresses are testnet addresses) or any other URL?

@hugoclrd

Copy link
Copy Markdown

@BowTiedRadone
Yes, this is the default API URL.
Why does rendezvous needs to know this URL? Does it fetch data itself?

Comment thread app.ts Outdated

@hugoclrd hugoclrd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think that rendezvous needs to set the default URL.
Does rendezvous itself need to make API calls? If not, it should probably let Clarinet do its things

@radubahmata

Copy link
Copy Markdown
Contributor Author

@BowTiedRadone Yes, this is the default API URL. Why does rendezvous needs to know this URL? Does it fetch data itself?

@hugoclrd Rendezvous needs a remote data object to start an empty simnet session. Here, the remote data didn’t specify an api_url. Since Rendezvous couldn’t find one, it used the default for missing remote data, while simnet defaulted to the mainnet API URL. This led to an inconsistency (Rendezvous could not deploy one of the contracts because the deployment plan expectedly did not include the requirements).

@hugoclrd

Copy link
Copy Markdown

Ok, I see.
So you can call initEmptySession() like so

await simnet.initEmptySession({ enabled: true })

So that you'll let Clarinet handle the default for the API URL, and more importantly, for the block height (which is dynamic - clarinet fetches the latest)

@radubahmata

Copy link
Copy Markdown
Contributor Author

@hugoclrd That makes sense, but only when the user does not explicitly specify an initial height and an API URL. For the rest of the cases, Rendezvous has to mimic the unit tests setup. This means I will remove this line, I will also remove the overwriting step and pass the object as-is. I'm pretty sure we still want to log the warning if enabled is false or does not exist (I guess it doesn't default to true if not specified but remote_data is present, right?).

@hugoclrd

Copy link
Copy Markdown

That makes sense, but only when the user does not explicitly specify an initial height and an API URL. For the rest of the cases, Rendezvous has to mimic the unit tests setup

Exactly 💯
That's the best way to mimic it, set the fields that are set in the settings, and omit the one that are not.

I'm pretty sure we still want to log the warning if enabled is false or does not exist (I guess it doesn't default to true if not specified but remote_data is present, right?).

Not sure in which case you'd want to show a warning. If enabled is false, then remote data is disabled. It's the same as not setting it

Comment thread app.ts Outdated

@moodmosaic moodmosaic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@radubahmata
radubahmata force-pushed the fix/remote-data-parsing branch from a286e25 to 60122ab Compare September 25, 2025 10:16
@radubahmata radubahmata changed the title Use mainnet API url when remote_data.api_url is unset Simplify remote data parsing Sep 25, 2025
Comment thread app.tests.ts
Comment thread app.ts Outdated
Comment thread app.ts Outdated
Comment thread app.ts

@moodmosaic moodmosaic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>
@radubahmata
radubahmata merged commit 8cd12ff into master Sep 25, 2025
80 of 85 checks passed
@radubahmata
radubahmata deleted the fix/remote-data-parsing branch September 25, 2025 12:06
@hugoclrd

hugoclrd commented Sep 25, 2025

Copy link
Copy Markdown

Love the latest changes 👍

As a side note, rendezvous is one of the few maintained project that build a tool on top of the SDK (since 99% of the SDK users use it to write unit tests).

So if you ever think that it could expose more data, or that the API should be different, or that the settings management should be better, etc, please start by opening an issue on Clarinet so that we can try to fix upstream 🙏 We can release fixes pretty quickly.

Patching it in rendezvous should always be the last resort

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