Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: testnet faucet is broken #504

Closed
Mirko-von-Leipzig opened this issue Sep 23, 2024 · 9 comments
Closed

[Bug]: testnet faucet is broken #504

Mirko-von-Leipzig opened this issue Sep 23, 2024 · 9 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Mirko-von-Leipzig
Copy link
Contributor

Mirko-von-Leipzig commented Sep 23, 2024

Requesting tokens from the testnet faucet no longer works.

This is due to the faucet code not persisting state, which caused the faucet to reset to initial state when the instance was restarted for maintenance. This causes the faucet's transactions to be rejected by the node which in turn means no tokens can be minted.

The best (imo) solution is for the faucet to sync its state on startup. This is possible because the faucet account is public. The convenient way to do this is blocked by #485. Until that is implemented our options are:

  1. Sync from genesis on startup
  2. Manually inject the state
  3. Restart the entire testnet network (and wait for [Task]: endpoint for account state retrieval (FPI) #485)
@Mirko-von-Leipzig Mirko-von-Leipzig added the bug Something isn't working label Sep 23, 2024
@igamigo
Copy link
Collaborator

igamigo commented Sep 23, 2024

I don't think the faucet account is currently set to be public, so syncing wouldn't solve it (unless we want to make it so). Another quick fix could be to persist the account state as a binary file and pick it up on startup instead of creating a new account each time.
I think currently the faucet always assumes its local state is valid so, at least validating it with a sync should be a good idea, but maybe separate to fixing this bug.

@Mirko-von-Leipzig
Copy link
Contributor Author

I don't think the faucet account is currently set to be public, so syncing wouldn't solve it (unless we want to make it so). Another quick fix could be to persist the account state as a binary file and pick it up on startup instead of creating a new account each time. I think currently the faucet always assumes its local state is valid so, at least validating it with a sync should be a good idea, but maybe separate to fixing this bug.

Ah you're right. I thought the faucet was using the faucet account deployed as part of genesis - which is public according to the default configuration.

Yeah if we keep it private then one has to persist to disk. Its a pity; having less state to worry about would have been nice :)

Also means the current faucet is completely dead.

@Mirko-von-Leipzig
Copy link
Contributor Author

Mirko-von-Leipzig commented Sep 23, 2024

Where is the faucet account actually sent to the node?

@igamigo
Copy link
Collaborator

igamigo commented Sep 23, 2024

For new accounts, the account seed is part of the transaction inputs and gets validated. Once the account is not new (ie, nonce != 0), this is not needed anymore. There's no such thing as deploying the account itself, if that's what you were referring to. In order to not have the client as a dependency, the faucet has an "ad-hoc" implementation of a DataStore which is needed to execute and send transactions.

@Mirko-von-Leipzig
Copy link
Contributor Author

Testnet faucet has been updated to use a different account ID essentially resetting the faucet without having to reset the network. Thanks @igamigo!

The proper fix can wait until #485 is complete.

@bobbinth
Copy link
Contributor

As a part of the proper fix, we can:

  • Make the faucet account public. This would prevent situations like this, but may also add complexity as it would let anyone on the network execute transactions against the faucet. There are two downsides to this:
    • Someone could try to "drain" the faucet by withdrawing all possible assets from it.
    • Our faucet code would need to make sure it has the latests faucet state before executing transactions (otherwise, if someone else executes a transaction against the faucet, the local state will be stale).
  • Add persistent storage to the faucet. The main downside is that we'd need to introduce database (or some other way) to keep track of account state.

Looking at the above, the second option is probably the "path of least resistance". We could try the first option too - but it'll probably be quite a bit more of work.

@igamigo
Copy link
Collaborator

igamigo commented Sep 23, 2024

As a part of the proper fix, we can:

  • Make the faucet account public. This would prevent situations like this, but may also add complexity as it would let anyone on the network execute transactions against the faucet. There are two downsides to this:

    • Someone could try to "drain" the faucet by withdrawing all possible assets from it.
    • Our faucet code would need to make sure it has the latests faucet state before executing transactions (otherwise, if someone else executes a transaction against the faucet, the local state will be stale).

How is the first point different to what occurs nowadays? If I understood correctly, only the account's public key would be public, right? The SecretKey would remain private to the faucet and so people wouldn't be able to execute transactions since they won't be able to provide signatures appropriately. BTW we wouldn't necessarily need to wait for #485 to validate the local state since we can use SyncState and/or GetAccountState endpoints, but this is probably too much overhead and the new endpoint would be simpler to use anyway.

  • Add persistent storage to the faucet. The main downside is that we'd need to introduce database (or some other way) to keep track of account state.

I think this could be as simple as saving the account's storage state (or maybe even just the first slot) in a file on each request and then loading it up on startup, right?

@bobbinth
Copy link
Contributor

How is the first point different to what occurs nowadays? If I understood correctly, only the account's public key would be public, right?

Ah - correct! I forgot that the faucet uses signature authentication (I thought it didn't have any authentication at all).

I think this could be as simple as saving the account's storage state (or maybe even just the first slot) in a file on each request and then loading it up on startup, right?

I'd also store the nonce - but yeah, these are the only things that are needed.

@polydez
Copy link
Contributor

polydez commented Oct 30, 2024

Resolved by #517

@polydez polydez closed this as completed Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

4 participants