feat: add ERC-8004 HTX parsing/validation and consolidate contract client utilities#62
feat: add ERC-8004 HTX parsing/validation and consolidate contract client utilities#62
Conversation
f1bc1d4 to
8e79e4c
Compare
6ae450a to
befe7e1
Compare
befe7e1 to
ec6856a
Compare
blacklight-node/src/main.rs
Outdated
| // Parse the HTX data - UnifiedHtx automatically detects provider field | ||
| let verification_result = match serde_json::from_slice::<Htx>(&event.rawHTX) { | ||
| // Debug: log the raw HTX bytes | ||
| let raw_bytes: &[u8] = &event.rawHTX; |
There was a problem hiding this comment.
Nit: Won't raw_bytes be unused if debug isn't set? Doesn't clippy complain here? I might be wrong, not sure how tracing exptects things.
There was a problem hiding this comment.
Yes, this is solved in the follow up PR.
| let rpc_url = config.rpc_url.clone(); | ||
| let ws_url = rpc_url | ||
| .replace("http://", "ws://") | ||
| .replace("https://", "wss://"); | ||
| let ctx = ProviderContext::with_ws_retries( | ||
| &config.rpc_url, | ||
| &private_key, | ||
| Some(config.max_ws_retries), | ||
| ) | ||
| .await?; | ||
|
|
||
| // Build WS transport with configurable retries | ||
| let ws = WsConnect::new(ws_url).with_max_retries(config.max_ws_retries); | ||
| let signer: PrivateKeySigner = private_key.parse::<PrivateKeySigner>()?; | ||
| let wallet = EthereumWallet::from(signer); | ||
|
|
||
| // Build a provider that can sign transactions, then erase the concrete type | ||
| let provider: DynProvider = ProviderBuilder::new() | ||
| .wallet(wallet.clone()) | ||
| .with_simple_nonce_management() | ||
| .with_gas_estimation() | ||
| .connect_ws(ws) | ||
| .await? | ||
| .erased(); |
There was a problem hiding this comment.
Why are we changing this? Seems orthogonal to the ERC-8004 changes.
There was a problem hiding this comment.
This comes related to your previous comment on removing the common/ subpath. I created a new crate to integrate with ERC 8004 smart contracts. That crate uses the same common modules to submit transactions and interact with the chain. In order to avoid repetitions in the code, I moved this to its own crate that is shared by the two other crates. Another reason to make this code common is: when you interact with a smart contract to submit transactions, the nonce is fundamental for transactions to be accepted, thus the use of the same lock is necessary to avoid transactions frontrunning others. That's why I thought the best idea was to unify the interfaces in ProviderContext.
| /// ERC-8004 Validation HTX data parsed from ABI-encoded bytes. | ||
| /// Format: `abi.encode(validatorAddress, agentId, requestURI, requestHash)` | ||
| #[derive(Debug, Clone)] | ||
| pub struct Erc8004Htx { |
There was a problem hiding this comment.
Nit: should we call this V1 as well?
There was a problem hiding this comment.
I don't think there can be a V2 for this unless the standard changes. I think it can be fine so far just having a single version.
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(tag = "provider", rename_all = "camelCase")] | ||
| pub enum Htx { | ||
| pub enum JsonHtx { |
There was a problem hiding this comment.
Maybe TeeHtx or do want to have any JSON here?
There was a problem hiding this comment.
I chose JSON as it is the format you parse the HTX from. The ERC 8004 comes from another smart contract invocation, and the format is not JSON but rather ABI encoded. I just differentiate how these are parsed and not the logical unit for their verification.
| #[test] | ||
| fn test_decode_error_string() { | ||
| // "blacklight: unknown HTX" encoded as Error(string) | ||
| // "NilAV: unknown HTX" encoded as Error(string) |
| // Selector: 08c379a0 | ||
| // Offset: 0000...0020 (32 bytes) | ||
| // Length: 0000...0012 (18 bytes = "blacklight: unknown HTX".len()) | ||
| // Length: 0000...0012 (18 bytes = "NilAV: unknown HTX".len()) |
There was a problem hiding this comment.
I don't know how this slipped. I am checking through the commits again because this came from a miss on a rebase.
There was a problem hiding this comment.
Oh, I see you added the mod here. Is this change necessary?
docker-compose.yml
Outdated
| # Anvil - Local Ethereum testnet | ||
| anvil: | ||
| image: ghcr.io/nillionnetwork/blacklight-contracts/anvil:sha-64cd680 | ||
| image: nilanvil:latest |
There was a problem hiding this comment.
You're right. This is not correct. Because I created multiple PRs on top of each other, this is solved in the follow up PR.
docker-compose.yml
Outdated
| networks: | ||
| - blacklight-network | ||
| command: ["--accounts", "15"] # Define the number of accounts to create | ||
| command: ["--accounts", "20"] # Define the number of accounts to create |
There was a problem hiding this comment.
since this is local maybe less accounts might be better to be more lightweight?
There was a problem hiding this comment.
This is a good point, I am going to reduce the number.
| impl Erc8004Htx { | ||
| /// Try to decode ABI-encoded ERC-8004 validation data. | ||
| pub fn try_decode(data: &[u8]) -> Result<Self, Erc8004DecodeError> { | ||
| let tuple_type = DynSolType::Tuple(vec![ |
There was a problem hiding this comment.
Is maybe using SolType instead of DynSolType make this easier? I'd hope alloy provides a better way of doing this than trying to decode each tuple field by hand.
There was a problem hiding this comment.
I've applied the change on the top-most PR to avoid having to re-rebase everything, but it is a great suggestion. Thanks!
| Err(anyhow::anyhow!( | ||
| "Transfer event not found in transaction receipt" | ||
| )) |
There was a problem hiding this comment.
nit: bail! (from anyhow) is a shorthand for this. e.g. bail!("beep failed")
There was a problem hiding this comment.
Same here, I've applied the change to the top-most PR to avoid having to rebase everything.
fix: ERC 8004 PR fixes
chore: restructured node
…responses feat: ERC 8004 validations responses
|
I've applied all the suggested changes and gone through everything to make sure it is all working fine. I know it may be hard to detect among such an amount of code, but they're all applied. I am merging, but if we find any issues with this implementation, I will go back to it. |
This PR introduces support for verifying agents submited through ERC 8004. This works together with blacklight-contracts#4.
tx_submitterin a different crate.simulator/.