feat: Implement u5c submit interface#76
Conversation
|
Note that this interface isn't useful right now, because it only works if you pass in a complete TX and the txbuilder only builds partial TXs. I don't think tx3 will change that, based on santi's sample code |
IIUC, the submit module is precisely for passing a complete tx. Tx building would be left to the client. Particularly, I added a function to pass a complete tx through the jsonrpc and submiting it that way. One could also send a Tx for it to be signed by a worker signer. |
|
Right, this interface is for passing a complete TX. I'm saying that because of the APIs balius exposes to the worker, it'll only have a complete TX to submit if that TX was passed into the worker from outside. So this interface lets people write an endpoint like the one you wrote, which takes a transaction and submits it. But there's no way for the worker to build and submit transactions, which is why I'm saying it isn't useful. If the caller has a signed transaction and a demeter key, they probably have access to a "submit transaction" API already, and balius isn't buying them much. |
Why is there no way for the worker to build the transaction? I understand a tx builder would make it easier, but its doable now. Even so, this is the shape that the interface will take, with or without tx builder. Should we not implement it till we have a way of building txs? |
It's technically possible to build a TX without a TX builder, but it's hard enough to do that it's practically impossible; you'd end up writing your own TX builder inside of the worker, reimplementing balancing and fee estimation and everything else yourself.
Yeah, that's all I'm saying. I don't think there's a point to implementing the interface if there isn't a way to use it. |
|
Hi, i think this PR makes more sense right now considering that the idea is to include tx3 building (my understanding) . I am exploring the capabilities of balius and i think this is definitely useful. |
📝 WalkthroughWalkthroughAdds a SubmitHost wrapper and U5C submit client, wires submit interface into worker/runtime construction, adds submit_tx metrics and new submit-error WIT variant, and propagates submit error handling into SDK and daemon configuration. Changes
Sequence DiagramsequenceDiagram
participant Worker as Worker
participant SubmitHost as SubmitHost
participant Metrics as Metrics
participant U5C as U5C Client
Worker->>SubmitHost: submit_tx(cbor)
SubmitHost->>Metrics: submit_tx(worker_id)
Metrics->>Metrics: increment counter
alt Submit::U5C
SubmitHost->>U5C: submit_tx(cbor)
U5C->>U5C: send to utxorpc endpoint
alt Success
U5C-->>SubmitHost: Ok(())
else GrpcError(code=3)
U5C-->>SubmitHost: Err(Invalid)
else Other Error
U5C-->>SubmitHost: Err(Internal)
end
else Submit::Mock
SubmitHost->>SubmitHost: print hex & return Ok
else Submit::Custom
SubmitHost->>SubmitHost: lock custom Submitter and delegate
end
SubmitHost-->>Worker: Result<(), SubmitError>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@balius-runtime/src/metrics.rs`:
- Around line 82-85: The submit_tx metric is missing the finalizing .build()
call causing a compile error; update the meter initialization for submit_tx (the
let submit_tx = meter.u64_counter("submit_tx").with_description("Amount of
submit_tx calls per worker.")) to call .build() at the end so it becomes a
completed counter before the next declaration (similar to how
signer_sign_payload is initialized).
In `@examples/comprehensive/src/lib.rs`:
- Around line 167-172: The function submit_tx is missing its final closing brace
causing a syntax error; locate the submit_tx function (signature: fn
submit_tx(_: Config<MyConfig>, request: Params<SubmitTxParams>) ->
WorkerResult<()>) and add the missing `}` immediately after the Ok(()) return so
the function that decodes cbor (hex::decode(...).map_err(|_| Error::BadParams)?)
and calls balius_sdk::wit::balius::app::submit::submit_tx(&cbor)? is properly
closed before the next item (LedgerSearchUtxosParams).
🧹 Nitpick comments (5)
balius-runtime/src/metrics.rs (1)
227-231: Missing blank line between methods.The
submit_txmethod follows the same pattern as other metric methods, which is correct. However, there's a missing blank line betweensubmit_txandsigner_sign_payloadmethods.✨ Proposed fix for formatting consistency
pub fn submit_tx(&self, worker_id: &str) { self.submit_tx .add(1, &[KeyValue::new("worker", worker_id.to_owned())]); } + pub fn signer_sign_payload(&self, worker_id: &str) {baliusd/src/main.rs (1)
14-25: Consider returningResultinstead of panicking on initialization failure.The
.expect()call will panic if the u5c client initialization fails. While this occurs at daemon startup, propagating the error would allow for more graceful error handling and consistent error reporting via the existingmiettediagnostics infrastructure used elsewhere in this file.♻️ Proposed refactor
impl Config { - pub async fn into_submit(&self) -> balius_runtime::submit::Submit { + pub async fn into_submit(&self) -> miette::Result<balius_runtime::submit::Submit> { match &self.submit { - Some(SubmitConfig::U5c(cfg)) => balius_runtime::submit::Submit::U5C( + Some(SubmitConfig::U5c(cfg)) => Ok(balius_runtime::submit::Submit::U5C( balius_runtime::submit::u5c::Submit::new(cfg) .await - .expect("Failed to convert config into submit interface"), - ), - None => balius_runtime::submit::Submit::Mock, + .into_diagnostic() + .context("setting up submit interface")?, + )), + None => Ok(balius_runtime::submit::Submit::Mock), } } }And at line 113:
- .with_submit(config.into_submit().await) + .with_submit(config.into_submit().await?)examples/comprehensive/src/lib.rs (1)
161-165: Consider adding#[serde_as]annotation for consistency.Other param structs in this file use
#[serde_as]annotation even when they don't have custom serialization. For consistency, consider adding it here, or alternatively remove it from structs that don't need it.balius-runtime/src/submit/u5c.rs (1)
36-44: Consider using a named constant for the gRPC status code.The hardcoded value
3representsINVALID_ARGUMENTin gRPC status codes. Using a named constant or the tonic/gRPC library's code enum would improve readability and maintainability.♻️ Proposed refactor
+// gRPC status code for INVALID_ARGUMENT +const GRPC_INVALID_ARGUMENT: i32 = 3; + .map_err(|err| match err { utxorpc::Error::GrpcError(status) => { let code: i32 = status.code().into(); - if code == 3 { + if code == GRPC_INVALID_ARGUMENT { wit::SubmitError::Invalid(status.to_string()) } else { wit::SubmitError::Internal(status.to_string()) } }balius-runtime/src/submit/mod.rs (1)
34-46: Consider counting successes separately from attempts.
submit_txincrements the counter before the backend call, so failures are included. If the metric is meant to represent successful submissions, consider moving the increment after a successful call (or add a failure counter / rename to attempts).🔧 Optional adjustment to count only successes
async fn submit_tx(&mut self, tx: wit::Cbor) -> Result<(), wit::SubmitError> { - self.metrics.submit_tx(&self.worker_id); - match &mut self.submit { + let result = match &mut self.submit { Submit::Mock => { println!("{}", hex::encode(tx)); Ok(()) } Submit::U5C(x) => x.submit_tx(tx).await, Submit::Custom(x) => { let mut lock = x.lock().await; lock.submit_tx(tx).await } - } + }; + if result.is_ok() { + self.metrics.submit_tx(&self.worker_id); + } + result }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@balius-runtime/src/lib.rs`:
- Around line 68-69: The Error::Submit enum variant in lib.rs is dead — it is
never constructed or converted from the runtime's internal submit errors (the
submit module returns wit::SubmitError) — so either remove the Submit(String)
variant from the Error enum or add a conversion that maps wit::SubmitError into
Error::Submit; to fix, update the Error definition in lib.rs (the Error enum)
and either delete Submit(String) or implement From<wit::SubmitError> for Error
(or adjust places in the submit module to convert/propagate wit::SubmitError
into Error::Submit) so the variant is actually used.
🧹 Nitpick comments (2)
balius-runtime/src/metrics.rs (1)
229-233: Nit: Missing blank line aftersubmit_txmethod.For consistency with the rest of the file, consider adding a blank line between the
submit_txandsigner_sign_payloadmethods.🧹 Proposed formatting fix
pub fn submit_tx(&self, worker_id: &str) { self.submit_tx .add(1, &[KeyValue::new("worker", worker_id.to_owned())]); } + pub fn signer_sign_payload(&self, worker_id: &str) {balius-runtime/src/submit/u5c.rs (1)
36-47: Consider using a named constant for gRPC status code.The magic number
3representsINVALID_ARGUMENTin gRPC. Using a named constant would improve readability and maintainability.📝 Suggested improvement
+// gRPC status code for INVALID_ARGUMENT +const GRPC_INVALID_ARGUMENT: i32 = 3; + pub async fn submit_tx(&mut self, tx: wit::Cbor) -> Result<(), wit::SubmitError> { self.client .submit_tx(vec![tx]) .await .map_err(|err| match err { utxorpc::Error::GrpcError(status) => { let code: i32 = status.code().into(); - if code == 3 { + if code == GRPC_INVALID_ARGUMENT { wit::SubmitError::Invalid(status.to_string()) } else { wit::SubmitError::Internal(status.to_string()) } }Alternatively, if
tonicis available, you could usetonic::Code::InvalidArgumentdirectly.
5 participants
Summary by CodeRabbit
New Features
Improvements