Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[NOT TESTED] Backend,Frontend,Tests: rm txDetail from SignedTx #224

Closed
wants to merge 3 commits into from
Closed

[NOT TESTED] Backend,Frontend,Tests: rm txDetail from SignedTx #224

wants to merge 3 commits into from

Conversation

aarani
Copy link
Contributor

@aarani aarani commented Oct 5, 2023

No description provided.

@aarani aarani force-pushed the updateNeth-squashed branch 9 times, most recently from da85353 to 2dbc20b Compare October 5, 2023 14:17
@knocte
Copy link
Member

knocte commented Oct 6, 2023

CI still red (but only one lane left)

@aarani aarani force-pushed the updateNeth-squashed branch 2 times, most recently from 98099b3 to ca42b5b Compare October 9, 2023 10:35
knocte
knocte reviewed Oct 9, 2023
View reviewed changes
src/GWallet.Backend/Ether/EtherServer.fs
Config.DEFAULT_NETWORK_TIMEOUT + Config.DEFAULT_NETWORK_TIMEOUT
| _ ->
Config.DEFAULT_NETWORK_TIMEOUT

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarani I dont see this explained in the commit msg

@aarani aarani changed the title Backend,Frontend,Tests: rm txDetail from SignedTx [NOT TESTED] Backend,Frontend,Tests: rm txDetail from SignedTx Oct 9, 2023
@aarani aarani force-pushed the updateNeth-squashed branch from ca42b5b to 760144e Compare October 9, 2023 12:42
@knocte
Copy link
Member

knocte commented Oct 11, 2023

Unfortunately this PR needs to be rebased. BTW, @aarani re-re-ping on the timeout-config removal.

@aarani aarani force-pushed the updateNeth-squashed branch 2 times, most recently from c689c85 to ebd9d66 Compare October 11, 2023 09:59
@knocte
Copy link
Member

knocte commented Oct 12, 2023

Weird that I'm seeing this CI error very often lately:

Secret source: None
Prepare workflow directory
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v1' (SHA:50fbc622fc4ef5163becd7fab6573eac35f8462e)
Download action repository 'actions/[email protected]' (SHA:51f68377c181a79065c61bd492bd49be4575c439)
Error: An action could not be found at the URI 'https://api.github.com/repos/actions/setup-dotnet/zipball/51f68377c181a79065c61bd492bd49be4575c439' (A481:7EF9:[33](https://github.com/nblockchain/geewallet/actions/runs/6480938090/job/17599192799#step:1:37)7E67:4C8CD7:65267EF7)

Can you please:

  • Re-run the failed CI job that has the error above. I think it can become green.
  • Create a PR that upgrades our checkout and setup-dotnet actions to latest.

@knocte
Copy link
Member

knocte commented Oct 12, 2023

It's noteworthy to say that Web3 no longer accepts a timeout as an argument.

Also:

  • Let's identify the commit of Nethereum/Nethereum repo that removes this argument. Maybe they are adding an alternative API in other class or method. I don't think we can merge this PR until we make sure we don't regress wrt timeouts.
  • Now that I think about it, this commit also changes things that are not explained; for example the Legacy things that are being used. I think we should include info in the commit about this (explaining that the old tx way of transactions are considered legacy because the new way of doing txs is the EIP thing, but we cannot adopt that because of ETC).

@aarani aarani force-pushed the updateNeth-squashed branch 3 times, most recently from 6ce7ee4 to 765f724 Compare October 12, 2023 09:56
aarani added 3 commits October 12, 2023 13:27
@aarani
Backend/Ether: rm etcIsMomentarilyCentralized flag
3a7bc4b 
ETC was previously marked as non-centeralized[1], this commit
removes this flag since it's no longer needed.

[1] 9392215
@aarani
Backend: update Nethereum to latest version
1985c81 
This commit updates Nethereum to latest version to support
extracting token transfer amount from the raw tx created when
sending (hot-storage) or signing (cold-storage).

It's noteworthy to say that Web3 no longer accepts a timeout
as an argument.

Nethereum now supports different types of transactions like
those that support EIP-1559, at this moment there's no
clear benefit for migrating especially given the fact that
it will causes incompatiblity with ETC so we will continue
to use Legacy types for now.
@aarani
Backend,Frontend,Tests: rm txDetail from SignedTx
c2d1892 
This commit removes the redundant proposal and metadata
from the SignedTransaction type.
@aarani aarani force-pushed the updateNeth-squashed branch from fb78f2c to 3a7bc4b Compare October 12, 2023 09:58
@knocte knocte force-pushed the master branch 2 times, most recently from 1a3be72 to 265b103 Compare October 16, 2023 05:05
aarani added a commit to aarani/geewallet that referenced this pull request Oct 19, 2023
@aarani
GithubCI: upgrade actions to latest version
0a03f2d 
Installing git before checkout was necessary to prevent this
error when git wants to interact with the checked out repo
(Probably because it's downloaded using Github's REST API):
```
fatal: not a git repository (or any parent up to mount point /)
```

Installing git before checkout also removes the problem with
using submodule parameter in checkout action so the comments we
had are no longer applicable.

This was done to fix errors we had with downloading actions [1].

[1] nblockchain#224 (comment)
@aarani aarani mentioned this pull request Oct 19, 2023
Open
@aarani
Copy link
Contributor Author

aarani commented Oct 19, 2023

Superseded by #238

@aarani aarani closed this Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knocte knocte knocte left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aarani @knocte