Test reserve and fallback builder transaction submission via private mempool #1970
Conversation
shenkeyao
requested review from
nomaxg,
philippecamacho,
ImJeremyHe,
sveitser,
jbearer,
tbro and
imabdulbasit
as code owners
| @@ -221,7 +223,7 @@ impl BuilderConfig { | |||
| let bid_config = bid_config.expect("Missing bid config for the reserve builder."); | |||
| let hooks = Arc::new(hooks::EspressoReserveHooks { | |||
| namespaces: bid_config.namespaces.into_iter().collect(), | |||
| solver_api_url, | |||
| solver_base_url, | |||
There was a problem hiding this comment.
Good change! This name is clearer.
| let private_key = | ||
| <BLSPubKey as SignatureKey>::PrivateKey::generate(&mut rand::thread_rng()); | ||
| let signature_key = BLSPubKey::from_private(&private_key); |
There was a problem hiding this comment.
It might just be how GitHub is displaying the diff, but these seems like the same logic that was at line 381? What is the purpose of the logic here?
There was a problem hiding this comment.
signature_keys is the list of keys that can update the registration, while the signature_key without "s" is the key to sign this registration. Alternatively, I could just create a list of keys and use one of them as the signature_key, but I was using the same implementation as in the mock solver test.
Now that I think about this, this helper function could be helpful for both builder and solver tests, so I'm going to simplify the logic here and move the function to the solver crate. (Update: It turns out that using this helper in the solver crate will require some refactoring not needed by the builder tests, so I'll just keep it in the current crate.)
There was a problem hiding this comment.
Alternatively, I could just create a list of keys and use one of them as the
signature_key, but I was using the same implementation as in the mock solver test.
I realized that this alternative approach wasn't really better than the current implementation, since we would need to keep track of the index in the list and make a copy of the private key associated with signature_key. The current implementation, though looking a bit redundant, actually does the minimal work needed. I'll just keep it as is unless you think it could be optimized. 😃
Closes #1966, closes #1967.
This PR:
Key places to review:
marketplace-builder/src/builder.rs.How to test this PR: