-
Notifications
You must be signed in to change notification settings - Fork 0
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
Hyperspace #4
base: master
Are you sure you want to change the base?
Hyperspace #4
Conversation
.env.local-hyperspace
Outdated
## simple-hub, xstate-wallet | ||
CONSENSUS_APP_ADDRESS = '0xeB1170bf49fac482fA296C98Ac04024e8a2d6519' | ||
|
||
### NitroAdjudicator and ETHAssetHolder deployed by George Knee, by uploading files to https://remix.ethereum.org/ and using compiler 0.6.10+commit.00c0fcaf, optimization ON. Source code verified on https://goerli.etherscan.io/ . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove stale comments (copypasta) and references to Goerli.
// TODO: This lets us get around CORS restrictions when using different ports on localhost | ||
if (corsUrl.startsWith('http://localhost')) { | ||
corsUrl = '*'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this a little more?
// TODO: Devtools doesn't support hyperspace yet | ||
if (Number(process.env.CHAIN_NETWORK_ID) === 3141) { | ||
process.env.TARGET_NETWORK = 'hyperspace'; | ||
console.log("Using 'hyperspace' as the target network"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much work to fixup devtools? We make this change in many places...
@@ -347,7 +347,7 @@ export class ChainWatcher implements Chain { | |||
|
|||
const response = await this.signer.sendTransaction({ | |||
...convertNitroTransactionRequest(transactionRequest), | |||
gasPrice: GAS_PRICE | |||
maxPriorityFeePerGas: GAS_PRICE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Is this a good way to migrate the transaction type? Here's a good reference https://www.liquity.org/blog/how-eip-1559-changes-the-transaction-fees-of-ethereum .
GAS_PRICE
is defined on L244 with some reference to Goerli network.
In the short term, we can do whatever seems to work. But we should have an eye on things like this which may break once deployed to Filecoin mainnet.
I think one alternative is to query the chain first to get a maxPriorityFeePerGas
.
✅ Deploy Preview for aesthetic-granita-d5edc2 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
- I am not sure why we get these messages - the resulting error is fixed in an upstream version of the package (thru dependence on client-api-schema, where bug was fixed) - this seems like a very harmless workaround for now
ignore mesages with data = ""
This should allow us to use the persistent seeder with hyperspace
This reverts commit 7a1090a.
No description provided.