-
Notifications
You must be signed in to change notification settings - Fork 478
feat: return sdk response with more fields inc the signer #6036
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
base: main
Are you sure you want to change the base?
Conversation
|
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.
If we want to do add this in a breaking way to TxClient, I would prefer to wait until v7. Conduit uses these endpoints
| ErrorLog string | ||
| Codespace string | ||
| GasWanted int64 | ||
| GasUsed int64 |
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.
do we want to add signer in the execution error as well?
Created the v2 wrapper @evan-forbes feel free to take a look at my approach. Once we bump the SDK I'll take another pass thru and open the PR. |
| defaultTmConfig := testnode.DefaultTendermintConfig() | ||
| defaultTmConfig.Mempool.TTLNumBlocks = ttlNumBlocks |
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] this isn't the defaultTmConfig any more if the function overrides a variable
| defaultTmConfig := testnode.DefaultTendermintConfig() | |
| defaultTmConfig.Mempool.TTLNumBlocks = ttlNumBlocks | |
| tmConfig := testnode.DefaultTendermintConfig() | |
| tmConfig.Mempool.TTLNumBlocks = ttlNumBlocks |
| defaultBlobParams := blobtypes.DefaultParams() | ||
| defaultBlobParams.GovMaxSquareSize = squareSize |
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] same, no longer the default
| defaultBlobParams := blobtypes.DefaultParams() | |
| defaultBlobParams.GovMaxSquareSize = squareSize | |
| blobParams := blobtypes.DefaultParams() | |
| blobParams.GovMaxSquareSize = squareSize |
| WithModifiers(genesis.SetBlobParams(enc.Codec, defaultBlobParams)) | ||
| // WithSuppressLogs(false) | ||
|
|
||
| testnodeConfig.Genesis.ConsensusParams.Block.MaxBytes = blocksize |
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.
[question] is it not possible to override this via a With___ function after testnode.DefaultConfig()?
| // NewTxClient creates a new v2 TxClient wrapper around the provided TxClient | ||
| func NewTxClient(client *user.TxClient) *TxClient { | ||
| return &TxClient{ | ||
| TxClient: client, | ||
| } | ||
| } |
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.
could we wrap the new function as well so the creator doesn't have to create v1 then wrap?
so
package v2
func New(args) {
v1.New(args)
etc
Overview
Fixes #5945
Fixes #5992