feat: verify via tx or hash #31
Conversation
✅ Deploy Preview for op-verify canceled.
|
There was a problem hiding this comment.
We should update the readme with some example invocations
| Required: true, | ||
| Name: "network", | ||
| Aliases: []string{"n"}, | ||
| Usage: "Network name: ethereum, op, base (required unless --tx is used)", |
There was a problem hiding this comment.
Can be a separate PR, but we should support sepolia too
cmd/op-txverify/main.go
Outdated
| address = core.StripChainPrefix(address) | ||
| // Check if using new --tx flag or old --network/--safe/--nonce flags | ||
| if txInput != "" { | ||
| // New method: fetch by transaction hash or link |
There was a problem hiding this comment.
nit, would delete the "new" and "old" nomenclature since this won't be new forever
cmd/op-txverify/main.go
Outdated
| } | ||
|
|
||
| // Validate network | ||
| if network != "ethereum" && network != "op" && network != "base" { |
There was a problem hiding this comment.
Does safe maintain a mapping of all networks they support and their APIs? Would be nice if we could just dump in a JSON with metadata to parse to support a bunch of chains easily
cmd/op-txverify/main.go
Outdated
| // Check if using new --tx flag or old --network/--safe/--nonce flags | ||
| if txInput != "" { |
There was a problem hiding this comment.
nit, looks like a lot of duplicate code here compared to above
| } | ||
|
|
||
| func downloadAction(c *cli.Context) error { | ||
| txInput := c.String("tx") |
There was a problem hiding this comment.
Can we add some tests for the new functionality?
core/generate.go
Outdated
| // Extract the hash from the data | ||
| if len(apiTx.Data) >= 74 { | ||
| innerHash := "0x" + apiTx.Data[10:74] |
There was a problem hiding this comment.
What are the 10 and 74 constants from? Some constants explaining that would be nice
| // Convert string values to appropriate types | ||
| var value, safeTxGas, baseGas, gasPrice int | ||
| fmt.Sscanf(content.Value, "%d", &value) | ||
| fmt.Sscanf(content.SafeTxGas, "%d", &safeTxGas) | ||
| fmt.Sscanf(content.BaseGas, "%d", &baseGas) | ||
| fmt.Sscanf(content.GasPrice, "%d", &gasPrice) |
There was a problem hiding this comment.
This feels very likely to overflow an int, doesn't this mean we only support a max value of 2^31 wei, which is a tiny amount of ETH? Should use a bigger type for these values
| safeTx := &SafeTransaction{ | ||
| Safe: safeAddress, | ||
| SafeVersion: safeVersion, | ||
| Chain: int(chainID), |
There was a problem hiding this comment.
Similar overflow risk since chainID can be a uint256, in practice uint64 should be big enough though
| MainnetChainID: "https://safe-transaction-mainnet.safe.global", | ||
| OPMainnetChainID: "https://safe-transaction-optimism.safe.global", |
2 participants
Description
Updates the
onlineanddownloadmethods to support verification by providing a Safe tx hash or a link to a Safe tx in the UI.